Repository navigation
feat(api): trusted-proxy client address, and gate traceparent on it - #855
Conversation
…parent Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
Code coverage overviewLanguages: Go Go
View coverage of the 10 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
…oxy-client-ip-cvhpw2 # Conflicts: # CHANGELOG.md
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
2d9d17ff-db7b-4f41-b8c8-a81f4485f1a4
📒 Files selected for processing: 22
AGENTS.mdCHANGELOG.mdconfig.yamldocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdxinternal/api/clientip.gointernal/api/clientip_test.gointernal/api/router.gointernal/app/wire.gointernal/app/wire_ops.gointernal/auth/auth.gointernal/config/backends.gointernal/config/config.gointernal/config/proxies.gointernal/config/proxies_test.gointernal/observability/logger.gointernal/observability/request.gointernal/observability/request_test.gotests/integration/otel_test.gotests/integration/setup_test.go
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines: 5
.github/prompts/docs-review.md — auto-discovered
docs/src/content/docs/architecture.md — auto-discovered
AGENTS.md — auto-discovered
.github/copilot-instructions.md — auto-discovered
CONTRIBUTING.md — auto-discovered
📓 Path-based instructions: 7
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse Documentation *prose* — accuracy against the code, runnable examples, clarity, completeness — **and code↔docs sync** (code that changed...
📄 CodeRabbit inference engine (.github/prompts/docs-review.md)
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/architecture.mddocs/src/content/docs/reverse-proxy.mdx
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse The invariant index — what must stay true.
📄 CodeRabbit inference engine (docs/src/content/docs/architecture.md)
Files:
internal/app/wire.gointernal/observability/logger.gointernal/config/config.gointernal/observability/request_test.gointernal/observability/request.go
Source excerpt: **Observability invariants** — stdout always 100% (sampling is OTLP-push-only); WARN+ERROR always export at 100% (a non-configurable floor — don't expose it); gRPC OTel exporters dial lazily so an unreachable collector never...
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/observability/logger.gointernal/observability/request_test.gointernal/observability/request.go
Source excerpt: **Bearer-token-only CORS posture (security)** — Bearer JWT on every request, no cookies/sessions; `corsMiddleware` deliberately **never** emits `Access-Control-Allow-Credentials` (not needed, and `*` + credentials is a spec...
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/api/router.go
Source excerpt: **Sealed MQ boundary** — only `internal/mq` imports NATS/JetStream (`github.com/nats-io/…`), enforced by the `depguard` rule in `.golangci.yml`, so `make lint` fails on a leak in every package it builds, including the files...
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/config/backends.gointernal/app/wire.gointernal/auth/auth.gointernal/observability/logger.gointernal/config/config.gointernal/observability/request_test.gointernal/config/proxies.gointernal/app/wire_ops.gointernal/api/clientip_test.gointernal/observability/request.gointernal/api/clientip.gointernal/api/router.gointernal/config/proxies_test.go
See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
AGENTS.md
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse **Scope** is the canonical docs-prose set from `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` EXCEPT `.claude/**`, `.github/**`, `CHAN...
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/deployment.mdAGENTS.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/architecture.mddocs/src/content/docs/reverse-proxy.mdxCHANGELOG.md
🧠 Learnings: 4
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-19T15:11:00.499Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Code Conventions
- **Go 1.26**, strict formatting (`gofumpt`, enforced by CI)
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-19T15:11:00.499Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
Source-of-truth pairs that must agree:
- Config struct tags in `internal/config/config.go` ↔ `docs/src/content/docs/configuration.mdx`, `config.yaml`, compose env blocks, `docs/src/content/docs/deployment.md`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-19T15:11:00.499Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
3. **Every code change updates its docs + `CHANGELOG.md` in the same PR** — a code change without its doc update is incomplete ([§Documentation Sync](#documentation-sync)).
📚 Learning: 2026-07-07T12:38:12.052Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:12.052Z
Learning: In this repo, do not add or recommend logging/tracing client IP addresses using naive or untrusted sources (e.g., `r.RemoteAddr` or directly trusting/deriving `X-Forwarded-For`) anywhere in the Go codebase. `middleware.RealIP` was removed due to IP-spoofing risks, and proper trusted-proxy-aware client-IP handling is intentionally deferred to issue #333. During code review, if proposed changes would record client IPs (including in audit paths such as `internal/auth/auth.go`), reject/redirect until #333 lands with correct trusted-proxy configuration and safeguards.
Applied to files:
internal/observability/request.gointernal/api/clientip.go
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
internal/config/proxies_test.go
🪛 LanguageTool
docs/src/content/docs/architecture.md
[style] ~138-~138: Since ownership is already implied, this phrasing may be redundant.
Context: ...endisembedded, dedupe.lease plus its own ceiling to the next whole second (ceil...
(PRP_OWN)
[style] ~200-~200: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...ogger in it too, so stdout carries them whether or not OTLP logs are on. otlpSamplerFn is ex...
(WHETHER)
[style] ~201-~201: Since ownership is already implied, this phrasing may be redundant.
Context: ...ds and spans alike; a record that names its own tenant keeps it. They are never metric ...
(PRP_OWN)
docs/src/content/docs/reverse-proxy.mdx
[style] ~201-~201: Since ownership is already implied, this phrasing may be redundant.
Context: ...arded-Proto/Host`** — set these for your own logs and any upstream that reads them. ...
(PRP_OWN)
[style] ~222-~222: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ..., ingress pods ``` How the client is chosen. When the connection comes from a lis...
(EN_REPEATEDWORDS_CHOOSE)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
8d6743cc-138e-43ff-9193-5c1d9809da88
📒 Files selected for processing: 5
CHANGELOG.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/reverse-proxy.mdxinternal/config/proxies.gointernal/config/proxies_test.go
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines: 3
.github/prompts/docs-review.md — auto-discovered
AGENTS.md — auto-discovered
CONTRIBUTING.md — auto-discovered
📓 Path-based instructions: 9
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse Documentation *prose* — accuracy against the code, runnable examples, clarity, completeness — **and code↔docs sync** (code that changed...
📄 CodeRabbit inference engine (.github/prompts/docs-review.md)
Files:
docs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
Source excerpt: Source-of-truth pairs that must agree: Config struct tags in `internal/config/config.go` ↔ `docs/src/content/docs/configuration.mdx`, `config.yaml`, compose env blocks, `docs/src/content/docs/deployment.md`
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/configuration.mdx
Source excerpt: Three invariants the docs site enforces in code, each of which fails quietly rather than loudly if you hand-write around it: **Opt a page into the Cloud CTA with `cloudCta` frontmatter**, not by importing the component.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
Source excerpt: **Table-driven tests**: Use `tests := []struct{ name string; ...
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/config/proxies_test.go
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse **Scope** is the canonical docs-prose set from `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` EXCEPT `.claude/**`, `.github/**`, `CHAN...
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
CHANGELOG.mddocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
Source excerpt: **In MDX, leave a blank line between a JSX tag and a code fence.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
Source excerpt: **Go 1.26**, strict formatting (`gofumpt`, enforced by CI)
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/config/proxies_test.gointernal/config/proxies.go
Source excerpt: **Editors see WH001 in `.md` only.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
CHANGELOG.md
Source excerpt: **Never hard-wrap prose.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
CHANGELOG.mddocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
🪛 LanguageTool
docs/src/content/docs/reverse-proxy.mdx
[style] ~236-~236: Since ownership is already implied, this phrasing may be redundant.
Context: ...72.17.0.1`), and a proxy container from its own address on that network. To find the ad...
(PRP_OWN)
🔇 Additional comments: 4
internal/config/proxies.go: 1
4-4: LGTM!Also applies to: 22-22, 29-29, 35-35, 42-42, 44-49
internal/config/proxies_test.go: 1
28-28: LGTM!docs/src/content/docs/configuration.mdx: 1
162-162: LGTM!CHANGELOG.md: 1
15-15: LGTM!Also applies to: 30-30
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
|
|
@coderabbitai review Generated by Claude Code |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Apply environment overrides after reading YAML. · config.go:388-399
internal/config/config.go:388-399
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winApply environment overrides after reading YAML.
When a YAML file exists,
Loadcallscleanenv.ReadConfigbut does not callcleanenv.ReadEnv. Therefore,WH_SERVER_TRUSTED_PROXIESis ignored when both sources are present. The YAML trusted-proxy policy is used instead of the documented environment override. Movecleanenv.ReadEnv(&cfg)after the YAML branch.Suggested fix
if err := rejectUnknownKeys(path); err != nil { return nil, err } - } else { - if err := cleanenv.ReadEnv(&cfg); err != nil { - return nil, fmt.Errorf("read env: %w", err) - } } + if err := cleanenv.ReadEnv(&cfg); err != nil { + return nil, fmt.Errorf("read env: %w", err) + }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
5e67dd6b-01b3-4298-8870-f6e1b72f171c
📒 Files selected for processing: 5
docs/src/content/docs/reverse-proxy.mdxinternal/api/clientip.gointernal/api/clientip_test.gointernal/observability/request.gointernal/observability/request_test.go
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines: 4
.github/prompts/docs-review.md — auto-discovered
docs/src/content/docs/architecture.md — auto-discovered
AGENTS.md — auto-discovered
CONTRIBUTING.md — auto-discovered
📓 Path-based instructions: 8
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse Documentation *prose* — accuracy against the code, runnable examples, clarity, completeness — **and code↔docs sync** (code that changed...
📄 CodeRabbit inference engine (.github/prompts/docs-review.md)
Files:
docs/src/content/docs/reverse-proxy.mdx
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse The invariant index — what must stay true.
📄 CodeRabbit inference engine (docs/src/content/docs/architecture.md)
Files:
internal/observability/request_test.gointernal/observability/request.go
Source excerpt: **Schema-driven ingest** — `POST /v1/ingest?table={table}` takes flat JSON, validated against the discovered schema (unknown fields rejected, types/nullability enforced).
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/api/clientip_test.gointernal/api/clientip.go
Source excerpt: **Table-driven tests**: Use `tests := []struct{ name string; ...
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/observability/request_test.gointernal/api/clientip_test.go
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse **Scope** is the canonical docs-prose set from `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` EXCEPT `.claude/**`, `.github/**`, `CHAN...
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/reverse-proxy.mdx
Source excerpt: **Go 1.26**, strict formatting (`gofumpt`, enforced by CI) Source excerpt: **Structured logging** with `log/slog` (JSON handler), through the default logger: call `slog.InfoContext(ctx, …)` and its siblings (the context carr...
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/observability/request_test.gointernal/observability/request.gointernal/api/clientip_test.gointernal/api/clientip.go
Source excerpt: **WH001 applies to every tracked Markdown file, with no carve-out** — `AGENTS.md`, `CHANGELOG.md`, `.github/` CI docs and `.claude/` agent prompts included.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/reverse-proxy.mdx
Source excerpt: **In MDX, leave a blank line between a JSX tag and a code fence.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/reverse-proxy.mdx
🧠 Learnings: 4
📓 Common learnings
Learnt from: taitelee
URL: https://github.com/Wave-RF/WaveHouse/pull/855
Timestamp: 2026-10-20T01:12:56.567Z
Learning: In Wave-RF/WaveHouse, the unresolved RemoteAddr issue in internal/api/clientip.go affected the server span's client.address, not request-scoped logs. The request scope was never populated from X-Forwarded-For directly. internal/observability/request.go omits empty client addresses from logs, while AnnotateSpan must overwrite otelhttp's header-derived client.address with an empty string when the peer cannot be resolved.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T02:03:28.428Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
| Change | Files to update |
| ------ | --------------- |
| Add/modify API endpoint | `docs/src/content/docs/api.md`, `README.md` (if user-facing) |
| Add/modify boot config option | `docs/src/content/docs/configuration.mdx`, `config.yaml`, `deployments/compose/*` env blocks, `docs/src/content/docs/deployment.md` |
| Add/modify a settings-directory key (`config.json`, `roles.json`, `policies.json`, `pipes.json`) | `docs/src/content/docs/settings-directory.mdx` (table **and** the example block), `pkg/settings/seed/*.json`, `deployments/compose/settings/*.json`, `tests/e2e/fixtures/settings/*.json`, `config.yaml` (the `settings:` comment enumerates the keys) |
| Change architecture / add a package | `docs/src/content/docs/architecture.md`, `AGENTS.md` |
| Change ingest / event format | `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md` (CH schema) |
| Change deployment / Docker | ...
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T02:03:28.428Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Testing Conventions
- **Every new function should have corresponding test cases.** Run `make lint` and `make test` before considering work complete.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T02:03:28.428Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
3. **Every code change updates its docs + `CHANGELOG.md` in the same PR** — a code change without its doc update is incomplete ([§Documentation Sync](#documentation-sync)).
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
internal/observability/request_test.go
📚 Learning: 2026-08-13T12:17:52.620Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 470
File: docs/src/content/docs/reverse-proxy.mdx:137-144
Timestamp: 2026-08-13T12:17:52.620Z
Learning: For Wave-RF/WaveHouse documentation, verify claims about implementation control flow against the authoritative implementation source (for example, internal/auth/auth.go) rather than relying solely on docs/** content. Documentation may lag behind or paraphrase behavior, so control-flow claims should be confirmed in source code.
Applied to files:
docs/src/content/docs/reverse-proxy.mdx
📚 Learning: 2026-07-07T12:38:12.052Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:12.052Z
Learning: In this repo, do not add or recommend logging/tracing client IP addresses using naive or untrusted sources (e.g., `r.RemoteAddr` or directly trusting/deriving `X-Forwarded-For`) anywhere in the Go codebase. `middleware.RealIP` was removed due to IP-spoofing risks, and proper trusted-proxy-aware client-IP handling is intentionally deferred to issue #333. During code review, if proposed changes would record client IPs (including in audit paths such as `internal/auth/auth.go`), reject/redirect until #333 lands with correct trusted-proxy configuration and safeguards.
Applied to files:
internal/api/clientip.go
🔇 Additional comments: 5
internal/api/clientip.go: 1
113-117: LGTM!internal/observability/request.go: 1
65-67: LGTM!Also applies to: 106-108
internal/observability/request_test.go: 1
198-207: LGTM!internal/api/clientip_test.go: 1
123-125: LGTM!docs/src/content/docs/reverse-proxy.mdx: 1
224-224: LGTM!
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
|
@coderabbitai Answers to your review of a588661:
Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^(reverse-proxy\.mdx|deployment\.md|config\.go|go\.mod|cleanenv\.go)$'
ast-grep outline internal/config/config.go --match 'Load' --view expanded
rg -n -C 5 'address.*(read|empty)|client\.address|TCP' docs/src/content/docs/reverse-proxy.mdx docs/src/content/docs/deployment.md
sed -n '350,425p' internal/config/config.go
rg -n -C 2 'cleanenv|^replace' go.mod
version=$(awk '$1 == "github.com/ilyakaznacheev/cleanenv" {print $2; exit}' go.mod)
if [[ -n "$version" ]]; then
curl --fail --silent --show-error --max-time 30 "https://raw.githubusercontent.com/ilyakaznacheev/cleanenv/${version}/cleanenv.go" |
awk '/^func ReadConfig\(/ {show=1} show {print} show && /^}/ {exit}'
fiLength of output: 48170
I did not run tests. ✏️ Learnings added
🧠 Learnings usedYou are interacting with an AI system. |
|
@coderabbitai review Generated by Claude Code |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
ff7cf09f-128c-464d-bb41-6ce008be4999
📒 Files selected for processing: 2
docs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdx
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines: 3
.github/prompts/docs-review.md — auto-discovered
CONTRIBUTING.md — auto-discovered
AGENTS.md — auto-discovered
📓 Path-based instructions: 4
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse Documentation *prose* — accuracy against the code, runnable examples, clarity, completeness — **and code↔docs sync** (code that changed...
📄 CodeRabbit inference engine (.github/prompts/docs-review.md)
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdx
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse **Scope** is the canonical docs-prose set from `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` EXCEPT `.claude/**`, `.github/**`, `CHAN...
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdx
Source excerpt: **Never hard-wrap prose.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdx
Source excerpt: **In MDX, leave a blank line between a JSX tag and a code fence.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/reverse-proxy.mdx
🧠 Learnings: 2
📓 Common learnings
Learnt from: taitelee
URL: https://github.com/Wave-RF/WaveHouse/pull/855
Timestamp: 2026-10-20T01:12:56.567Z
Learning: In Wave-RF/WaveHouse, the unresolved RemoteAddr issue in internal/api/clientip.go affected the server span's client.address, not request-scoped logs. The request scope was never populated from X-Forwarded-For directly. internal/observability/request.go omits empty client addresses from logs, while AnnotateSpan must overwrite otelhttp's header-derived client.address with an empty string when the peer cannot be resolved.
Learnt from: taitelee
URL: https://github.com/Wave-RF/WaveHouse/pull/855
Timestamp: 2026-10-20T02:11:42.589Z
Learning: In Wave-RF/WaveHouse, internal/config/config.go Load uses github.com/ilyakaznacheev/cleanenv v1.5.0. cleanenv.ReadConfig parses the configuration file and then calls readEnvVars(cfg, false), so bound WH_* environment variables, including WH_SERVER_TRUSTED_PROXIES, already override YAML values. The separate cleanenv.ReadEnv call belongs in the no-file branch; moving it outside that branch would read the environment twice when a file exists.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T03:17:13.125Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
| Change | Files to update |
| ------ | --------------- |
| Any notable change | `CHANGELOG.md` under `[Unreleased]` |
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T03:17:13.125Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
Source-of-truth pairs that must agree:
- Config struct tags in `internal/config/config.go` ↔ `docs/src/content/docs/configuration.mdx`, `config.yaml`, compose env blocks, `docs/src/content/docs/deployment.md`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T03:17:13.125Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
### Markdown authoring rules
- **WH001 applies to every tracked Markdown file, with no carve-out** — `AGENTS.md`, `CHANGELOG.md`, `.github/` CI docs and `.claude/` agent prompts included. Path-scoped `.markdownlint.json` files under `.github/` and `.claude/` used to switch it off there; [#521](https://github.com/Wave-RF/WaveHouse/issues/521) deleted them, because a config that silently contradicts `CONTRIBUTING.md` teaches a reviewer reading the config the wrong invariant. Don't reintroduce one — if a file genuinely can't satisfy WH001, say why in a `markdownlint-disable` comment in the file itself, where the next reader sees it. This makes WH001 *broader* than `scripts/docs-prose.sh`, which still skips `.github/` and `.claude/`: mechanical style is cheap to enforce everywhere, a prose review is not.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T03:17:13.125Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
| Change | Files to update |
| ------ | --------------- |
| Add/modify boot config option | `docs/src/content/docs/configuration.mdx`, `config.yaml`, `deployments/compose/*` env blocks, `docs/src/content/docs/deployment.md` |
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T03:17:13.125Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
3. **Every code change updates its docs + `CHANGELOG.md` in the same PR** — a code change without its doc update is incomplete ([§Documentation Sync](#documentation-sync)).
📚 Learning: 2026-08-13T12:17:52.620Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 470
File: docs/src/content/docs/reverse-proxy.mdx:137-144
Timestamp: 2026-08-13T12:17:52.620Z
Learning: For Wave-RF/WaveHouse documentation, verify claims about implementation control flow against the authoritative implementation source (for example, internal/auth/auth.go) rather than relying solely on docs/** content. Documentation may lag behind or paraphrase behavior, so control-flow claims should be confirmed in source code.
Applied to files:
docs/src/content/docs/reverse-proxy.mdx
🔇 Additional comments: 2
docs/src/content/docs/deployment.md: 1
766-766: LGTM!docs/src/content/docs/reverse-proxy.mdx: 1
226-226: LGTM!
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
|
@coderabbitai review Generated by Claude Code |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Qualify /0 trust by address family. · proxies.go:67-73
internal/config/proxies.go:67-73
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQualify
/0trust by address family.The matcher trusts a peer only when
p.Contains(a)succeeds. With only0.0.0.0/0, an IPv6 peer is not trusted, soclientAddrreturns the peer address without readingX-Forwarded-For. The warning and configuration documentation say “every peer,” which overstates the behavior and can mislead operators configuring a single-family proxy. Scope both statements to the entry’s address family.Suggested fix
--- a/internal/config/proxies.go +++ b/internal/config/proxies.go @@ -// trustedProxyWarnings flags a /0 entry: it trusts every peer, so a client -// reaching the port directly chooses the address it is logged under. +// trustedProxyWarnings flags a /0 entry: it trusts every peer in its address +// family, so a direct client in that family chooses the address it is logged under. @@ - return []string{"server.trusted_proxies includes " + p.String() + ": every peer is trusted, so a client that reaches server.port directly sets its own client.address through X-Forwarded-For and has its traceparent adopted; list only your proxies' addresses"} + return []string{"server.trusted_proxies includes " + p.String() + ": every peer in that address family is trusted, so a client in that family that reaches server.port directly sets its own client.address through X-Forwarded-For and has its traceparent adopted; list only your proxies' addresses"} --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -- **`server.trusted_proxies` with a `/0` entry** (`0.0.0.0/0`, `::/0`), in every process: every peer is trusted, so a client that reaches `server.port` directly picks its own `client.address` and joins its `traceparent` to your traces (see [Reverse proxy](/reverse-proxy#client-address-and-trace-context)). +- **`server.trusted_proxies` with a `/0` entry** (`0.0.0.0/0`, `::/0`), in every process: every peer in that entry's address family is trusted, so a client in that family that reaches `server.port` directly picks its own `client.address` and joins its `traceparent` to your traces (see [Reverse proxy](/reverse-proxy#client-address-and-trace-context)).
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
404355bc-0717-4c61-8bc0-5f85b2e9c98c
📒 Files selected for processing: 1
docs/src/content/docs/deployment.md
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines: 3
.github/prompts/docs-review.md — auto-discovered
CONTRIBUTING.md — auto-discovered
AGENTS.md — auto-discovered
📓 Path-based instructions: 4
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse Documentation *prose* — accuracy against the code, runnable examples, clarity, completeness — **and code↔docs sync** (code that changed...
📄 CodeRabbit inference engine (.github/prompts/docs-review.md)
Files:
docs/src/content/docs/deployment.md
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse **Scope** is the canonical docs-prose set from `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` EXCEPT `.claude/**`, `.github/**`, `CHAN...
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/deployment.md
Source excerpt: **Never hard-wrap prose.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/deployment.md
Cross-cutting habits that keep the codebase reviewable — they apply to every change, in every language.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/deployment.md
🧠 Learnings: 1
📓 Common learnings
Learnt from: taitelee
URL: https://github.com/Wave-RF/WaveHouse/pull/855
Timestamp: 2026-10-20T01:12:56.567Z
Learning: In Wave-RF/WaveHouse, the unresolved RemoteAddr issue in internal/api/clientip.go affected the server span's client.address, not request-scoped logs. The request scope was never populated from X-Forwarded-For directly. internal/observability/request.go omits empty client addresses from logs, while AnnotateSpan must overwrite otelhttp's header-derived client.address with an empty string when the peer cannot be resolved.
Learnt from: taitelee
URL: https://github.com/Wave-RF/WaveHouse/pull/855
Timestamp: 2026-10-20T02:11:42.589Z
Learning: In Wave-RF/WaveHouse, internal/config/config.go Load uses github.com/ilyakaznacheev/cleanenv v1.5.0. cleanenv.ReadConfig parses the configuration file and then calls readEnvVars(cfg, false), so bound WH_* environment variables, including WH_SERVER_TRUSTED_PROXIES, already override YAML values. The separate cleanenv.ReadEnv call belongs in the no-file branch; moving it outside that branch would read the environment twice when a file exists.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T04:20:32.371Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
Source-of-truth pairs that must agree:
- Config struct tags in `internal/config/config.go` ↔ `docs/src/content/docs/configuration.mdx`, `config.yaml`, compose env blocks, `docs/src/content/docs/deployment.md`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T04:20:32.371Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Testing Conventions
- **Every new function should have corresponding test cases.** Run `make lint` and `make test` before considering work complete.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T04:20:32.371Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
3. **Every code change updates its docs + `CHANGELOG.md` in the same PR** — a code change without its doc update is incomplete ([§Documentation Sync](#documentation-sync)).
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-20T04:20:32.371Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
### Markdown authoring rules
- **WH001 applies to every tracked Markdown file, with no carve-out** — `AGENTS.md`, `CHANGELOG.md`, `.github/` CI docs and `.claude/` agent prompts included. Path-scoped `.markdownlint.json` files under `.github/` and `.claude/` used to switch it off there; [#521](https://github.com/Wave-RF/WaveHouse/issues/521) deleted them, because a config that silently contradicts `CONTRIBUTING.md` teaches a reviewer reading the config the wrong invariant. Don't reintroduce one — if a file genuinely can't satisfy WH001, say why in a `markdownlint-disable` comment in the file itself, where the next reader sees it. This makes WH001 *broader* than `scripts/docs-prose.sh`, which still skips `.github/` and `.claude/`: mechanical style is cheap to enforce everywhere, a prose review is not.
🔇 Additional comments: 1
docs/src/content/docs/deployment.md: 1
766-766: LGTM!
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK
|
@coderabbitai Thanks, the outside-diff finding on
Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^(proxies\.go|proxies_test\.go|configuration\.mdx|clientip\.go)$'
ast-grep outline internal/config/proxies.go
ast-grep outline internal/config/proxies_test.go --match TestWarnings_TrustEveryPeer --view expanded
ast-grep outline internal/api/clientip.go
sed -n '1,140p' internal/config/proxies.go
rg -n -A65 -B3 'func TestWarnings_TrustEveryPeer\(' internal/config/proxies_test.go
rg -n -C4 '/0|every IPv4|every IPv6' docs/src/content/docs/configuration.mdx
sed -n '1,150p' internal/api/clientip.goLength of output: 10530
I did not run tests. ✏️ Learnings added
You are interacting with an AI system. |
|
@coderabbitai review Generated by Claude Code |
✅ Action performedReview finished.
|
…ls (#866) <!-- claude-projects-attribution: {"github_login":"taitelee"} --> _Requested by **Taite** · [project thread](https://claude.ai/code/project/chan_019fe1KaM5pLaH8s2bVcLf4T?thread=cmsg_019fe1KaM5pLaH8s2bVcLf4TDQK62Tr5rS3frnE6fVLTPT)_ ## Summary The first tier of #94. Before this, the only request metrics came from otelhttp. Those carry no route, so a slow or failing endpoint could not be told apart from the rest. Nothing timed the statements the query paths send to ClickHouse. - `wavehouse_http_request_duration_seconds{route, method, status_class}` times every request on `server.port` except SSE streams. The middleware lives in `newProbeRouter`, so both the API router and the ops-only router get it. It sits outside `jsonRecoverer`, so a panic counts as a `5xx`. `route` is the chi pattern (`/v1/pipes/{name}`), never the path. It is the mount's own pattern (`/v1/*`, `/v1/ops/*`) for a request a mount answers short of a route, and `unmatched` for one that reached no mount. `method` is net/http's or `other`. - `wavehouse_clickhouse_query_duration_seconds{operation, result}` times each statement sent by `POST /v1/query` (`query`), a pipe (`pipe`, `pipe_write`) or `POST /v1/ops/query` (`ops_query`). `result` is `ok`, `canceled` (the client went away), or the response's error code without its `clickhouse.` prefix, through the same `chFailureOf` the response uses. It is recorded inside the singleflight closure, so a cache hit records nothing and coalesced requests record once. - A histogram's `_count` gives both the rate and the error rate, so there is no separate `_errors_total`. Neither metric carries a tenant or table label. - The docs are in deployment.md § Request latency and ClickHouse queries and in architecture.md. The CHANGELOG and AGENTS.md are updated too. This touches `router.go` (one line), which #855 also edits. It should merge after #855. ## Test plan - [x] `internal/api/metrics_test.go` covers: - Route-pattern labeling: pattern not path, `/v1/*`, `/v1/ops/*`, `unmatched`. Panic as 5xx, `/v1/stream` skipped. - The ops router's wiring. - Every row of the ClickHouse error matrix, across `query`, `pipe` and `pipe_write`. - The proxy's ok, rejected and response-cap paths, plus a refused request recording nothing. - Client gone vs. the time-cap backstop. - Nil no-op. - [x] `go test -race ./internal/api/ ./internal/app/` and golangci-lint on `./internal/api/...` - [ ] GitHub CI ## Related Issues Part of #94, part of #777. The worker's `INSERT` timing, the next tier, waits on #770. --- _Generated by [Claude Code](https://claude.ai/code/session_019YVNGPfArHqjc3Xt6N9App)_ --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Requested by Taite · project thread
Summary
Before: behind a proxy, WaveHouse's logs only had the proxy's address, and the server span's
client.addresswas whatever the client wrote first inX-Forwarded-For(otelhttp reads the leftmost entry unconditionally). Any client'straceparentwas adopted as the parent of the request's span.After:
server.trusted_proxies(WH_SERVER_TRUSTED_PROXIES, CIDRs or bare addresses, default empty) lists the proxies to believe. From a listed peer,X-Forwarded-Foris walked right to left and the first hop that isn't a listed proxy is the client; from anyone else the client is the connection's own address and the header is ignored. The result isclient.addresson every request-scoped log record (operator-key audit line included) and on the server span, replacing otelhttp's value. Atraceparentis joined only from a listed proxy; from anyone else otelhttp starts a new root linked to it (WithPublicEndpointFn).r.RemoteAddris never rewritten.How:
internal/config/proxies.goparses and validates the list (bad entry refuses boot, a/0entry is a boot warning).internal/api/clientip.goholds the walk and aclientAddressmiddleware right afterrequestID;observability.SetRequestClientstores it on the existing request scope from #771, andAnnotateSpan/TraceHandleremit it. The otelhttp middleware is now built once per router instead of once per request.Behavior change: with the default empty list, a tracing proxy's
traceparentis now linked rather than continued until the proxy is listed. It has its own### ChangedCHANGELOG entry. The integration suite now lists loopback as trusted soTestOTel_IngestTraceContinuitystill joins the caller's trace.Call sites in files open PRs #712/#770 also edit: one line in
internal/app/wire.go(deps.TrustedProxies = …).wire_ops.gogets the same line.Test plan
make build,go vet ./...(and-tags integration ./tests/integration/)go test -race ./internal/api ./internal/config ./internal/observability;go test ./internal/app ./internal/authclient.addresson the span with the resolved value; traceparent joined from a trusted peer, linked (new root) from an untrusted oneRelated Issues
Closes #333
Part of #777. Reviewer noted a possible follow-up outside this scope:
baggageis still extracted from any peer (pre-existing; nothing reads it today).🤖 Generated with Claude Code
https://claude.ai/code/session_01UbPNcxn6XzeNyU3qBMohvK