feat(auth): non-JWT operator key for admin + break-glass access - #378
Conversation
Add an optional auth.operator_key (WH_AUTH_OPERATOR_KEY): a request presenting it in the X-Operator-Key header is authorized as a full-access platform operator -- the whole data plane and the /v1/admin/* management surface -- without minting a JWT and independently of the token verifier. The middleware checks it before the Bearer token with a constant-time comparison (crypto/subtle) and stamps two things into the request context: the live admin_role (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that RequireAdmin honors even when the policy is nil/deleted -- the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged. auth.Middleware now takes the policy store and logger by constructor injection, returning the plain middleware type (which also keeps the existing test call sites unchanged). Includes unit tests (auth, router, config), docs sync, and config samples. Closes #240. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughAdds a non-JWT operator key ( ChangesOperator Key Break-Glass Feature
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
|
📚 Docs preview is live → https://68228591-wavehouse-docs.wave-rf.workers.dev
|
Code Coverage OverviewLanguages: Go GoThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad9ca8a9-3111-4c21-afb6-871d0e4ed561
📒 Files selected for processing (19)
AGENTS.mdCHANGELOG.mdcmd/wavehouse/main.goconfig.yamldeployments/compose/standalone.yamldocs/src/content/docs/access-control.mdxdocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/development.mdinternal/api/router.gointernal/api/router_test.gointernal/auth/auth.gointernal/auth/auth_test.gointernal/auth/context.gointernal/config/config.gointernal/config/config_test.gotests/e2e/fixtures/config.yaml
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Docs preview
- GitHub Check: Coverage
- GitHub Check: E2E tests
- GitHub Check: Lint
⚠️ CI failures not shown inline (2)
GitHub Actions: PR housekeeping / PR housekeeping: feat(auth): non-JWT operator key for admin + break-glass access
Conclusion: failure
##[group]Run # Single source of truth for the rule: scripts/lint-pr-title.sh — the
�[36;1m# Single source of truth for the rule: scripts/lint-pr-title.sh — the�[0m
�[36;1m# SAME validator the local agent gate runs (.claude/hooks/agent-bash-gate.sh),�[0m
�[36;1m# so CI and local can't drift. The checkout above is ref: main, so this is�[0m
�[36;1m# always the default-branch script. Dependabot's grouped-update titles�[0m
�[36;1m# routinely exceed the 72-char subject cap and the format isn't�[0m
�[36;1m# configurable, so Dependabot PRs are exempt from the length check�[0m
�[36;1m# (the format check still applies).�[0m
�[36;1mif [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "app/dependabot" ]]; then�[0m
�[36;1m export PR_TITLE_SKIP_LENGTH=1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif reason=$(bash scripts/lint-pr-title.sh "$PR_TITLE" 2>&1); then�[0m
�[36;1m echo "passed=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1m echo "PR title OK: $PR_TITLE"�[0m
�[36;1melse�[0m
�[36;1m echo "passed=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1m printf '%s\n' "$reason"�[0m
�[36;1m echo "::error::$(printf '%s' "$reason" | head -1)"�[0m
GitHub Actions: PR housekeeping / 0_PR housekeeping.txt: feat(auth): non-JWT operator key for admin + break-glass access
Conclusion: failure
##[group]Run # Single source of truth for the rule: scripts/lint-pr-title.sh — the
�[36;1m# Single source of truth for the rule: scripts/lint-pr-title.sh — the�[0m
�[36;1m# SAME validator the local agent gate runs (.claude/hooks/agent-bash-gate.sh),�[0m
�[36;1m# so CI and local can't drift. The checkout above is ref: main, so this is�[0m
�[36;1m# always the default-branch script. Dependabot's grouped-update titles�[0m
�[36;1m# routinely exceed the 72-char subject cap and the format isn't�[0m
�[36;1m# configurable, so Dependabot PRs are exempt from the length check�[0m
�[36;1m# (the format check still applies).�[0m
�[36;1mif [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "app/dependabot" ]]; then�[0m
�[36;1m export PR_TITLE_SKIP_LENGTH=1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif reason=$(bash scripts/lint-pr-title.sh "$PR_TITLE" 2>&1); then�[0m
�[36;1m echo "passed=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1m echo "PR title OK: $PR_TITLE"�[0m
�[36;1melse�[0m
�[36;1m echo "passed=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1m printf '%s\n' "$reason"�[0m
�[36;1m echo "::error::$(printf '%s' "$reason" | head -1)"�[0m
🧰 Additional context used
📓 Path-based instructions (14)
deployments/compose/**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Compose files must reflect current config defaults and runtime settings when configuration or deployment behavior changes.
Files:
deployments/compose/standalone.yaml
docs/src/content/docs/deployment.md
📄 CodeRabbit inference engine (AGENTS.md)
Document deployment, Docker, and ingest-event/ClickHouse schema changes in the deployment docs when they affect runtime setup.
Files:
docs/src/content/docs/deployment.md
cmd/wavehouse/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
cmd/wavehouseis the standalone binary entrypoint and should remain thin wiring around the application.
Files:
cmd/wavehouse/main.go
docs/src/content/docs/development.md
📄 CodeRabbit inference engine (AGENTS.md)
Document changes to build or test process in the development docs.
Files:
docs/src/content/docs/development.md
internal/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/apiowns the Chi HTTP router, JWT/JWKS middleware, and handlers for ingest, query, structured query, SSE, schema, DLQ, policy, and pipes.
Files:
internal/api/router_test.gointernal/api/router.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Use table-driven tests witht.Run(tt.name, ...)and add corresponding test cases for every new function.
Prefer the shared helpers ininternal/testutil/for mocks, JWTs, schema registries, policy stores, pipes stores, and HTTP response assertions.
Files:
internal/api/router_test.gointernal/config/config_test.gointernal/auth/auth_test.go
internal/auth/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
JWT auth middleware must support either HMAC or JWKS verification, pin
algto the active verifier, extract roles from the configured claim path, and never hard-fail on bad tokens (bad token yields empty role plus stashed reason).
Files:
internal/auth/context.gointernal/auth/auth.gointernal/auth/auth_test.go
internal/config/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Configuration structs must use YAML/env/env-default tags and reflect the documented config surface.
Files:
internal/config/config_test.gointernal/config/config.go
internal/api/router.go
📄 CodeRabbit inference engine (AGENTS.md)
Route registrations in
router.gomust stay synchronized with the API docs, and/v1/*routes must continue to run JWT auth middleware.
Files:
internal/api/router.go
docs/src/content/docs/**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
docs/src/content/docs/**/*.mdx: Docs prose under the Starlight site must stay accurate to code, runnable examples, clear, and complete, with code↔docs sync maintained for changed behavior.
Mermaid diagrams in docs should default to vertical layouts, avoid wide side-by-side comparisons, and keep labels short for readability.
Files:
docs/src/content/docs/configuration.mdxdocs/src/content/docs/access-control.mdx
docs/src/content/docs/architecture.md
📄 CodeRabbit inference engine (AGENTS.md)
Document architectural changes or new packages in the architecture docs.
Files:
docs/src/content/docs/architecture.md
docs/src/content/docs/api.md
📄 CodeRabbit inference engine (AGENTS.md)
Document any new or changed API endpoint, route registration, error response, or ingest/event format in the API docs.
Files:
docs/src/content/docs/api.md
internal/config/config.go
📄 CodeRabbit inference engine (AGENTS.md)
Config struct tags in
internal/config/config.gomust match the documented config keys and env mapping.
Files:
internal/config/config.go
AGENTS.md
📄 CodeRabbit inference engine (AGENTS.md)
Update
AGENTS.mdwhen architecture or process changes require new agent instructions or repository invariants.
Files:
AGENTS.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-06T21:35:55.048Z
Learning: Validate locally before every push by running `make ci` the documented way.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-06T21:35:55.048Z
Learning: Before every PR-branch push, run `/prepush` so every required pre-push reviewer reaches `ship_it` or is deliberately skipped on the record.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-06T21:35:55.048Z
Learning: Every code change must update its corresponding docs and `CHANGELOG.md` in the same PR.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-06T21:35:55.048Z
Learning: Address and resolve every review finding; reply substantively, fix it or track it in an issue, mention the bot when required, and resolve the thread.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-06T21:35:55.048Z
Learning: Create agent PRs as drafts only, and ensure the PR title passes the Conventional Commits gate (≤ 72 chars, lowercase-first subject, no trailing period).
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-06T21:35:55.048Z
Learning: Never force-push or rebase a PR branch; merge upstream `main` instead.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-06T21:35:55.048Z
Learning: Never hand-write review markers or use `--no-verify`; use the documented gates and skip tooling instead.
📚 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:
docs/src/content/docs/deployment.mddocs/src/content/docs/development.mdCHANGELOG.mddocs/src/content/docs/architecture.mddocs/src/content/docs/api.mdAGENTS.md
📚 Learning: 2026-05-20T01:02:00.784Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:00.784Z
Learning: In WaveHouse’s internal API tests (files matching internal/api/**/*_test.go), follow the existing separation-of-concerns convention for testing the RequireRole middleware: inject `ContextKeyRole` directly into the request `context.Context` instead of using `testutil.MakeJWT`/JWT-driven flows. Do not refactor role-gate tests to use JWT tokens—JWT parsing and token handling are covered separately in `middleware_test.go` (the dedicated JWT parsing tests), and mixing those concerns would expand the failure surface and reduce isolation.
Applied to files:
internal/api/router_test.go
📚 Learning: 2026-05-23T01:23:59.268Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 174
File: internal/api/ingest_test.go:111-111
Timestamp: 2026-05-23T01:23:59.268Z
Learning: In WaveHouse Go tests in internal/api/**/*_test.go, use internal/testutil.AssertJSONErrorResponse(t, w) for HTTP error-path JSON assertions. Do not use (or reintroduce) package-local assertJSONErrorResponse helpers. AssertJSONErrorResponse verifies the response Content-Type is application/json, includes the X-Content-Type-Options: nosniff header, and that the JSON body contains an "error" field.
Applied to files:
internal/api/router_test.go
📚 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/api/router_test.gointernal/config/config_test.gointernal/auth/auth_test.go
🪛 Betterleaks (1.6.0)
docs/src/content/docs/development.md
[high] 214-214: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🔇 Additional comments (27)
cmd/wavehouse/main.go (2)
81-87: LGTM!
378-382: LGTM!internal/config/config.go (1)
152-163: LGTM!internal/config/config_test.go (1)
28-28: LGTM!Also applies to: 56-56, 70-78
internal/auth/auth.go (1)
107-116: LGTM!Also applies to: 128-137, 179-183
internal/api/router.go (1)
239-242: LGTM!Also applies to: 255-262
internal/api/router_test.go (1)
91-119: LGTM!internal/auth/context.go (1)
20-20: LGTM!Also applies to: 67-82
config.yaml (1)
90-90: LGTM!deployments/compose/standalone.yaml (1)
30-34: LGTM!tests/e2e/fixtures/config.yaml (1)
26-26: LGTM!internal/auth/auth_test.go (1)
8-56: LGTM!Also applies to: 73-76, 203-203, 215-215, 328-347
docs/src/content/docs/api.md (2)
32-39: Same scope-nuance issue as above: this paragraph repeats the broad “entire data plane” claim. Please narrow it to match the nil-policy break-glass behavior.
40-44: LGTM!docs/src/content/docs/configuration.mdx (3)
148-158: Same scope-nuance issue as above: this config table repeats the broad “whole data plane” claim. Please narrow it so it matches the nil-policy recovery semantics.
260-260: LGTM!
329-329: LGTM!AGENTS.md (1)
56-60: LGTM!CHANGELOG.md (1)
14-14: LGTM!docs/src/content/docs/access-control.mdx (3)
34-34: LGTM!
413-413: LGTM!
67-78: 🎯 Functional CorrectnessOperator-key scope is correct. The operator key is intentionally broad: it authorizes the entire data plane and
/v1/admin/*, and it still works when the policy is nil/deleted.> Likely an incorrect or invalid review comment.docs/src/content/docs/architecture.md (3)
72-72: LGTM!
96-96: LGTM!
202-202: LGTM!docs/src/content/docs/deployment.md (1)
139-142: LGTM!docs/src/content/docs/development.md (1)
209-216: LGTM!
github-project-automation
Bot
moved this from Backlog
to In review
in WaveHouse Task Board
Address CodeRabbit review on #378: - Consolidate the 8 operator-key middleware tests into one table-driven test with t.Run (repo test guideline); same scenarios and assertions, and still exercises both the logger != nil and logger == nil branches. - Add remote_addr to the operator-key audit log so break-glass use is attributable during an incident (trusted client-IP behind a proxy is #333). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
Address maintainer review on #378: - Accept the operator key in the standard Authorization header with an "Operator" scheme (Authorization: Operator <key>) — forwarded verbatim by proxies and free of any Bearer/JWT collision; keep X-Operator-Key as an alias. The Authorization header takes precedence and is matched case-insensitively (RFC 7235). - Revert the remote_addr field from the operator-key audit log. It was the only IP logged anywhere in WaveHouse and contradicts the deliberate no-IP stance (RealIP dropped in #332; trusted-proxy client-IP for logs is tracked in #333), and r.RemoteAddr behind a proxy is just the proxy's IP. The audit line will inherit a proper, trusted-proxy-aware client IP once #333 lands. Docs updated across api.md, access-control.mdx, configuration.mdx, reverse-proxy.mdx, deployment.md, development.md, architecture.md, AGENTS.md, the config samples, and CHANGELOG. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
Address review on #378 (the docs named both transports but a few code-level references and the security summary still lagged): - Startup INFO log, config.Auth.OperatorKey doc comment, and the auth.Config.OperatorKey field comment now name "Authorization: Operator <key>" (preferred) alongside the X-Operator-Key alias. - SECURITY.md: the operator key bypasses per-role policy (RBAC bullet) and reaches POST /v1/admin/query without a JWT and even under a deleted policy (query-passthrough bullet) — the access-model summary was missing the exception. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
…y line Address review on #378: - WithOperator doc comment (internal/auth/context.go) and the RequireAdmin inline comment (internal/api/router.go) now name both transports (Authorization: Operator <key>, or the X-Operator-Key alias) — the last two comments the previous sweep missed. - development.md "Validating tokens": a nil/unseeded policy denies every token-based caller (not "everyone") — the operator key still reaches the admin surface, matching the operator-key paragraph just below and the code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/auth/auth.go (1)
107-141: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider correlating the operator-key audit log with the request ID.
The audit log currently records only
path/method. Sincemiddleware.RequestIDis already wired inrouter.go, adding the chi request ID (not client IP — consistent with the repo's stance against logging untrusted IP sources) would make break-glass usage traceable across log lines/traces without reintroducing the IP-spoofing concern raised in the earlier review round.💡 Optional addition
+ reqID := chimw.GetReqID(r.Context()) logger.LogAttrs(r.Context(), slog.LevelInfo, "operator key authenticated request", slog.String("path", r.URL.Path), slog.String("method", r.Method), + slog.String("request_id", reqID), )Separately, worth confirming whether
internal/config/config.goenforces any minimum length/entropy onauth.operator_key— this credential grants full-access bypass of both JWT and the admin policy gate, so a short/weak value would be a much higher-impact secret to leak or brute-force than the JWT secret, which does get a dedicated weak-default warning inmain.go.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0bbc83e-0777-42c5-875d-99f3727e60c7
📒 Files selected for processing (19)
AGENTS.mdCHANGELOG.mdSECURITY.mdcmd/wavehouse/main.goconfig.yamldeployments/compose/standalone.yamldocs/src/content/docs/access-control.mdxdocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/development.mddocs/src/content/docs/reverse-proxy.mdxinternal/api/router.gointernal/auth/auth.gointernal/auth/auth_test.gointernal/auth/context.gointernal/config/config.gotests/e2e/fixtures/config.yaml
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Coverage
- GitHub Check: Docs build
- GitHub Check: E2E tests
- GitHub Check: Lint
⚠️ CI failures not shown inline (2)
GitHub Actions: PR housekeeping / 0_PR housekeeping.txt: feat(auth): non-JWT operator key for admin + break-glass access
Conclusion: failure
##[group]Run # Single source of truth for the rule: scripts/lint-pr-title.sh — the
�[36;1m# Single source of truth for the rule: scripts/lint-pr-title.sh — the�[0m
�[36;1m# SAME validator the local agent gate runs (.claude/hooks/agent-bash-gate.sh),�[0m
�[36;1m# so CI and local can't drift. The checkout above is ref: main, so this is�[0m
�[36;1m# always the default-branch script. Dependabot's grouped-update titles�[0m
�[36;1m# routinely exceed the 72-char subject cap and the format isn't�[0m
�[36;1m# configurable, so Dependabot PRs are exempt from the length check�[0m
�[36;1m# (the format check still applies).�[0m
�[36;1mif [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "app/dependabot" ]]; then�[0m
�[36;1m export PR_TITLE_SKIP_LENGTH=1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif reason=$(bash scripts/lint-pr-title.sh "$PR_TITLE" 2>&1); then�[0m
�[36;1m echo "passed=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1m echo "PR title OK: $PR_TITLE"�[0m
�[36;1melse�[0m
�[36;1m echo "passed=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1m printf '%s\n' "$reason"�[0m
�[36;1m echo "::error::$(printf '%s' "$reason" | head -1)"�[0m
GitHub Actions: PR housekeeping / PR housekeeping: feat(auth): non-JWT operator key for admin + break-glass access
Conclusion: failure
##[group]Run # Single source of truth for the rule: scripts/lint-pr-title.sh — the
�[36;1m# Single source of truth for the rule: scripts/lint-pr-title.sh — the�[0m
�[36;1m# SAME validator the local agent gate runs (.claude/hooks/agent-bash-gate.sh),�[0m
�[36;1m# so CI and local can't drift. The checkout above is ref: main, so this is�[0m
�[36;1m# always the default-branch script. Dependabot's grouped-update titles�[0m
�[36;1m# routinely exceed the 72-char subject cap and the format isn't�[0m
�[36;1m# configurable, so Dependabot PRs are exempt from the length check�[0m
�[36;1m# (the format check still applies).�[0m
�[36;1mif [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "app/dependabot" ]]; then�[0m
�[36;1m export PR_TITLE_SKIP_LENGTH=1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif reason=$(bash scripts/lint-pr-title.sh "$PR_TITLE" 2>&1); then�[0m
�[36;1m echo "passed=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1m echo "PR title OK: $PR_TITLE"�[0m
�[36;1melse�[0m
�[36;1m echo "passed=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1m printf '%s\n' "$reason"�[0m
�[36;1m echo "::error::$(printf '%s' "$reason" | head -1)"�[0m
🧰 Additional context used
📓 Path-based instructions (7)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Every code change must update the corresponding documentation andCHANGELOG.mdin the same PR.
Address and resolve every review finding; reply substantively, fix it or track it in an issue, mention the bot when required, and never silently drop a thread.
Files:
deployments/compose/standalone.yamldocs/src/content/docs/deployment.mdtests/e2e/fixtures/config.yamlCHANGELOG.mdinternal/config/config.goconfig.yamldocs/src/content/docs/reverse-proxy.mdxinternal/api/router.godocs/src/content/docs/access-control.mdxSECURITY.mddocs/src/content/docs/configuration.mdxinternal/auth/auth.goAGENTS.mdcmd/wavehouse/main.godocs/src/content/docs/api.mddocs/src/content/docs/architecture.mdinternal/auth/context.godocs/src/content/docs/development.mdinternal/auth/auth_test.go
docs/src/content/docs/{api,configuration,deployment,development,architecture}.md
📄 CodeRabbit inference engine (AGENTS.md)
Update the relevant docs page when changing API endpoints, config options, deployment behavior, build/test process, or architecture/package structure.
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/development.md
docs/src/content/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
docs/src/content/docs/**/*.{md,mdx}: Docs prose must stay accurate, complete, and in sync with code; follow the docs-review scope for all tracked docs except the documented exclusions.
Author Mermaid diagrams vertically by default (flowchart TB/TD), avoid large side-by-side diagrams, and keep node labels short so diagrams remain legible in the docs column width.
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/access-control.mdxdocs/src/content/docs/configuration.mdxdocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/development.md
CHANGELOG.md
📄 CodeRabbit inference engine (AGENTS.md)
Record any notable change under the
[Unreleased]section inCHANGELOG.md.
Files:
CHANGELOG.md
internal/config/config.go
📄 CodeRabbit inference engine (AGENTS.md)
Config struct tags in
internal/config/config.gomust stay in sync with configuration docs,config.yaml, and compose env blocks.
Files:
internal/config/config.go
internal/api/router.go
📄 CodeRabbit inference engine (AGENTS.md)
Route registrations in
internal/api/router.gomust stay in sync with the API docs, and all/v1/*routes must continue to run the JWT auth middleware.
Files:
internal/api/router.go
docs/src/content/docs/architecture.md
📄 CodeRabbit inference engine (AGENTS.md)
When changing a core package or repository architecture, update
docs/src/content/docs/architecture.mdand keep the named invariant intact.
Files:
docs/src/content/docs/architecture.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T12:38:00.597Z
Learning: On PR branches, run the full pre-push self-review flow (`/prepush`) and ensure every required reviewer reaches `ship_it` or a logged skip before pushing.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T12:38:00.597Z
Learning: Agents must create draft PRs only, use a Conventional Commits–compliant title of at most 72 characters, and validate the title with `scripts/lint-pr-title.sh` before creating the PR.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T12:38:00.597Z
Learning: Never force-push or rebase a PR branch; merge `origin/main` instead when syncing with upstream.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T12:38:00.597Z
Learning: Never hand-write review markers or use `--no-verify`; use the prescribed validation and skip tooling instead.
📚 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:
docs/src/content/docs/deployment.mdCHANGELOG.mdSECURITY.mdAGENTS.mddocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/development.md
📚 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/config/config.gointernal/api/router.gointernal/auth/auth.gocmd/wavehouse/main.gointernal/auth/context.gointernal/auth/auth_test.go
📚 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/auth/auth_test.go
🔇 Additional comments (22)
AGENTS.md (1)
56-60: LGTM!CHANGELOG.md (1)
14-14: LGTM!SECURITY.md (1)
28-30: LGTM!docs/src/content/docs/development.md (1)
203-218: LGTM!docs/src/content/docs/reverse-proxy.mdx (1)
139-140: 🔒 Security & PrivacyVerify
Authorizationsurvives the full proxy chain.This wording assumes every ingress/auth layer forwards
Authorizationunchanged. Please verify the actual proxy stack here; some layers consume or rewrite that header, which would break the new operator-key transport.docs/src/content/docs/access-control.mdx (1)
34-34: LGTM!Also applies to: 67-78, 413-413
docs/src/content/docs/api.md (1)
32-36: LGTM!docs/src/content/docs/architecture.md (2)
202-202: LGTM!
72-72: 🎯 Functional CorrectnessAlign the operator-bit name here with line 96 — this uses
auth.IsOperator, while the other doc text refers toauth.WithOperator; use one term consistently or clarify getter vs. setter.docs/src/content/docs/configuration.mdx (1)
148-148: LGTM!Also applies to: 154-158, 260-260, 329-329
docs/src/content/docs/deployment.md (1)
139-140: LGTM!internal/config/config.go (1)
152-163: LGTM!internal/auth/context.go (1)
14-20: LGTM!Also applies to: 67-83
config.yaml (1)
86-91: LGTM!deployments/compose/standalone.yaml (1)
26-35: LGTM!tests/e2e/fixtures/config.yaml (1)
26-26: LGTM!internal/auth/auth.go (2)
18-23: LGTM!Also applies to: 51-58
184-198: LGTM!internal/auth/auth_test.go (1)
8-56: LGTM!Also applies to: 73-80, 207-207, 219-219, 247-387, 389-408
internal/api/router.go (1)
46-192: LGTM! The operator bypass is correctly additive —deps.AuthMWstill runs for the whole/v1subtree and JWT verification remains intact. As per path instructions, "all/v1/*routes must continue to run the JWT auth middleware," which is satisfied here.Also applies to: 248-267
Source: Path instructions
cmd/wavehouse/main.go (2)
375-386: LGTM!
81-86: 🔒 Security & PrivacyCheck
auth.operator_keystrength validation. Add a minimum-length warning here ifinternal/config/config.godoesn’t already enforce it; this key bypasses JWT and restores full operator access.
…eneration Address review questions on #378: - Rewrite operatorKey() using strings.Cut instead of len()+slice+EqualFold, so it reads like bearerToken() while keeping the RFC 7235 case-insensitive scheme match. Behavior-preserving — all operator-key tests pass unchanged. - Docs: add an operator-key generation snippet (openssl rand / /dev/urandom) and a curl example to access-control.mdx's Operator key section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
…r + Operator Per review on #378: extract authScheme(r, scheme) so operatorKey and bearerToken parse the Authorization header identically, and make the "Bearer" scheme case-insensitive too (RFC 7235 auth-schemes are case-insensitive; only the canonical "Bearer" casing was accepted before). A JWT still has to fully validate, so accepting a case variant of the scheme has no security impact. Adds TestMiddleware_BearerScheme_CaseInsensitive (bearer / BEARER / BeArEr) as a regression guard, alongside the existing operator-scheme case-insensitive case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
…c nits Per CodeRabbit review on #378: - Keep the operator-key audit log at path/method: request-scoped correlation (request_id, and the eventual trusted-proxy client IP) belongs in the global TraceHandler (internal/observability), not sprinkled per call site — folded into #333. Comment updated to say so. - architecture.md: refer to "the operator bit" consistently (was auth.IsOperator on one line, auth.WithOperator on another). - reverse-proxy.mdx: soften "proxies forward Authorization unchanged" — an auth-terminating layer that consumes/rewrites the header is the exception. No minimum-length check on operator_key (consistent with #325, closed not_planned); the docs already show `openssl rand -base64 32`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
|
@coderabbitai thanks — addressing your walkthrough points from the
Also FYI the CI failure your review captured was stale (from an earlier commit) — — Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '60,100p' docs/src/content/docs/architecture.md
echo "---reverse-proxy---"
sed -n '1,40p' docs/src/content/docs/reverse-proxy.mdx
echo "---auth.go audit comment---"
rg -n "request_id|TraceHandler|audit" internal/auth/auth.goLength of output: 12275 All four points check out against current head — architecture.md line 72 says "the operator bit" (matches the setter reference), reverse-proxy.mdx section on Authorization forwarding is present, and auth.go's audit comment now points to the global
CI status noted — nothing further from me here. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/auth/auth.go (1)
117-118: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse a fixed-length comparison for the operator key.
subtle.ConstantTimeComparestill returns early on length mismatch, so this leaks whether the presented key matches the configured length. Hash both sides first or enforce a fixed-length secret.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b3afaee6-d5a6-4c21-8997-f153f7c9a194
📒 Files selected for processing (6)
CHANGELOG.mddocs/src/content/docs/access-control.mdxdocs/src/content/docs/architecture.mddocs/src/content/docs/reverse-proxy.mdxinternal/auth/auth.gointernal/auth/auth_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
docs/src/content/docs/**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
Author Mermaid diagrams vertically by default (
flowchart TB/TD), keep labels short, and avoid wide side-by-side diagrams unless the chain is genuinely short.
Files:
docs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/access-control.mdx
internal/auth/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In JWT auth middleware, the verifier must always run, accept either HMAC or JWKS (not both), pin the accepted
algto the active verifier, rejectalg:noneand cross-family confusion, and map no/invalid tokens to the default role with a stashed reason.
Files:
internal/auth/auth.gointernal/auth/auth_test.go
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Validate locally before every push by running `make ci` the documented way; do not use CI as the first feedback loop.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Before pushing a PR branch, run `/prepush`, satisfy every required pre-push reviewer from `scripts/pre-push-reviewers.sh`, and loop until each reviewer returns `ship_it`.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Every code change must update its corresponding documentation and `CHANGELOG.md` in the same PR.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Address and resolve every review finding; reply substantively, fix it or track it in an issue, mention the bot when required, and never silently drop a thread.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Agents must create draft PRs only, and the PR title must pass the Conventional Commits lint rule (≤72 chars) before creation.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Never force-push or rebase a PR branch; absorb upstream changes with `git merge origin/main`.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Never hand-write review markers or use `--no-verify`; use the prescribed gates and marker tooling instead.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T13:52:04.520Z
Learning: Use `make help` as the source of truth for available targets and documented tooling behavior.
📚 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.mddocs/src/content/docs/architecture.md
📚 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/auth/auth.gointernal/auth/auth_test.go
📚 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/auth/auth_test.go
🔇 Additional comments (7)
CHANGELOG.md (1)
14-14: LGTM!docs/src/content/docs/access-control.mdx (1)
34-34: LGTM!Also applies to: 67-92, 427-427
docs/src/content/docs/architecture.md (1)
72-72: LGTM!Also applies to: 96-96, 202-202
docs/src/content/docs/reverse-proxy.mdx (1)
139-140: LGTM!internal/auth/auth.go (2)
22-22: LGTM!Also applies to: 58-103, 119-143, 203-208, 215-217
190-192: 🎯 Functional CorrectnessAccept one-or-more spaces after the auth scheme.
strings.Cut(..., " ")can leave leading spaces incred, soAuthorization: bearer <jwt>may fail.internal/auth/auth_test.go (1)
107-123: LGTM!
taitelee
left a comment
taitelee
left a comment
There was a problem hiding this comment.
We can probably add a metric and log for operator key failed attempts but besides that looks pretty straightforward.
A request presenting an operator credential that does not match the
configured key is now logged at WARN ("operator key authentication
failed") and counted by a new wavehouse_auth_operator_key_failures_total
OTel counter, then falls through to the normal Bearer/default path — the
middleware still never rejects. A wrong operator key is never sent by
accident (legitimate callers present a Bearer JWT or nothing), so a
mismatch is a probing/brute-force signal against the most privileged
credential in the system, and is now observable and alertable.
An absent operator credential is not a failed attempt: the WARN + counter
fire only when a non-empty operator credential was presented but did not
match, so ordinary traffic never drowns the signal. The constant-time
compare runs only on a non-empty credential (whether a header was sent is
not secret). Correlation fields (request_id / client IP) remain deferred
to the global TraceHandler (#333), matching the success audit line.
Addresses the non-blocking reviewer suggestion on #240 (PR #378).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE
4d8c865
|
@taitelee thanks for the review — good call on the failed-attempt observability. Implemented in
Heads-up: this new commit dismissed your approval under last-push-approval, so it'll need a fresh 👍 when you get a moment. — Claude Code |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/content/docs/architecture.md (1)
202-202: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winInconsistent terminology: "operator-key bit" vs. "operator bit".
Lines 72 and 96 use "operator bit" for the context field, but this line says "the operator-key bit" instead. The PR follow-up specifically fixed this file's wording to consistently say "the operator bit" — this instance appears to have been missed.
📝 Proposed fix
- → /v1/admin RequireAdmin (role == policy.admin_role, or the operator-key bit) — single gate shared + → /v1/admin RequireAdmin (role == policy.admin_role, or the operator bit) — single gate sharedBased on the PR comments summary noting "Documentation wording in
architecture.mdwas fixed to consistently say 'the operator bit.'"
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c8acfc22-4b5f-4bd1-9ff1-125daf306e84
📒 Files selected for processing (6)
CHANGELOG.mdSECURITY.mddocs/src/content/docs/access-control.mdxdocs/src/content/docs/architecture.mdinternal/auth/auth.gointernal/auth/auth_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: E2E tests
- GitHub Check: Coverage
- GitHub Check: Integration tests
- GitHub Check: Docs build
- GitHub Check: Lint
⚠️ CI failures not shown inline (2)
GitHub Actions: PR housekeeping / 0_PR housekeeping.txt: feat(auth): non-JWT operator key for admin + break-glass access
Conclusion: failure
##[group]Run # Single source of truth for the rule: scripts/lint-pr-title.sh — the
�[36;1m# Single source of truth for the rule: scripts/lint-pr-title.sh — the�[0m
�[36;1m# SAME validator the local agent gate runs (.claude/hooks/agent-bash-gate.sh),�[0m
�[36;1m# so CI and local can't drift. The checkout above is ref: main, so this is�[0m
�[36;1m# always the default-branch script. Dependabot's grouped-update titles�[0m
�[36;1m# routinely exceed the 72-char subject cap and the format isn't�[0m
�[36;1m# configurable, so Dependabot PRs are exempt from the length check�[0m
�[36;1m# (the format check still applies).�[0m
�[36;1mif [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "app/dependabot" ]]; then�[0m
�[36;1m export PR_TITLE_SKIP_LENGTH=1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif reason=$(bash scripts/lint-pr-title.sh "$PR_TITLE" 2>&1); then�[0m
�[36;1m echo "passed=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1m echo "PR title OK: $PR_TITLE"�[0m
�[36;1melse�[0m
�[36;1m echo "passed=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1m printf '%s\n' "$reason"�[0m
�[36;1m echo "::error::$(printf '%s' "$reason" | head -1)"�[0m
GitHub Actions: PR housekeeping / PR housekeeping: feat(auth): non-JWT operator key for admin + break-glass access
Conclusion: failure
##[group]Run # Single source of truth for the rule: scripts/lint-pr-title.sh — the
�[36;1m# Single source of truth for the rule: scripts/lint-pr-title.sh — the�[0m
�[36;1m# SAME validator the local agent gate runs (.claude/hooks/agent-bash-gate.sh),�[0m
�[36;1m# so CI and local can't drift. The checkout above is ref: main, so this is�[0m
�[36;1m# always the default-branch script. Dependabot's grouped-update titles�[0m
�[36;1m# routinely exceed the 72-char subject cap and the format isn't�[0m
�[36;1m# configurable, so Dependabot PRs are exempt from the length check�[0m
�[36;1m# (the format check still applies).�[0m
�[36;1mif [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "app/dependabot" ]]; then�[0m
�[36;1m export PR_TITLE_SKIP_LENGTH=1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif reason=$(bash scripts/lint-pr-title.sh "$PR_TITLE" 2>&1); then�[0m
�[36;1m echo "passed=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1m echo "PR title OK: $PR_TITLE"�[0m
�[36;1melse�[0m
�[36;1m echo "passed=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1m printf '%s\n' "$reason"�[0m
�[36;1m echo "::error::$(printf '%s' "$reason" | head -1)"�[0m
🧰 Additional context used
📓 Path-based instructions (5)
CHANGELOG.md
📄 CodeRabbit inference engine (AGENTS.md)
Record notable changes under the
[Unreleased]section.
Files:
CHANGELOG.md
**/*.{go,mod,sum}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{go,mod,sum}: Use Go 1.26 conventions: strict formatting, structured logging withlog/slog, Chi v5 for HTTP routing, explicit dependency injection, and error returns with wrapping instead of panics.
Keep package names lowercase and single-word (or abbreviated), and avoid global state by passing dependencies explicitly.
Files:
internal/auth/auth.gointernal/auth/auth_test.go
internal/auth/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Keep JWT authentication always on and fail-loud: verify with HMAC or JWKS (not both), pin the accepted
algto the active verifier, and preserve the empty-role fallback plus stashed bad-token reason.
Files:
internal/auth/auth.gointernal/auth/auth_test.go
docs/src/content/docs/architecture.md
📄 CodeRabbit inference engine (AGENTS.md)
Document architecture changes and new packages in the architecture guide, keeping invariant descriptions aligned with the code.
Files:
docs/src/content/docs/architecture.md
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Write table-driven tests with
t.Run, keep tests in the same package as the code under test, and add corresponding tests for every new function.
Files:
internal/auth/auth_test.go
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T16:02:19.911Z
Learning: Validate locally before every push by running `make ci` the documented way; do not use CI as the first feedback loop.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T16:02:19.911Z
Learning: On PR-branch pushes, run the required pre-push reviewers discovered by `scripts/pre-push-reviewers.sh` (via `/prepush`) until each returns `ship_it`; do not hardcode the reviewer set.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T16:02:19.911Z
Learning: Every code change must update its corresponding documentation and `CHANGELOG.md` in the same PR.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T16:02:19.911Z
Learning: Address and resolve every review finding; reply substantively, fix it or track it in an issue, and never silently drop a comment.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T16:02:19.911Z
Learning: Agents must create draft PRs only, and the PR title must pass the Conventional Commits gate and stay within 72 characters.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T16:02:19.911Z
Learning: Never force-push or rebase a PR branch; merge `origin/main` instead when syncing with upstream.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-07T16:02:19.911Z
Learning: Never hand-write review markers or use `--no-verify`; use the documented gates and skip tooling instead.
📚 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:
SECURITY.mdCHANGELOG.mddocs/src/content/docs/architecture.md
📚 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/auth/auth.gointernal/auth/auth_test.go
📚 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/auth/auth_test.go
🔇 Additional comments (8)
internal/auth/auth.go (3)
32-41: LGTM!
169-186: LGTM!
133-168: 🔒 Security & PrivacyBreak-glass audit still depends on a non-nil logger. If
cfg.OperatorKeycan be configured withlogger == nil, successful operator-key use emits no audit line and failures lose the WARN as well. Ensure the middleware is always called with a logger in that configuration.internal/auth/auth_test.go (1)
408-473: LGTM!CHANGELOG.md (1)
14-14: LGTM!SECURITY.md (1)
28-30: LGTM!docs/src/content/docs/access-control.mdx (1)
73-94: LGTM!Also applies to: 429-429
docs/src/content/docs/architecture.md (1)
72-72: LGTM!Also applies to: 96-96
github-project-automation
Bot
moved this from In progress
to Done
in WaveHouse Task Board
2 participants
Summary
Adds an optional non-JWT operator key (
auth.operator_key/WH_AUTH_OPERATOR_KEY, presented in theX-Operator-Keyheader) for the person running a WaveHouse deployment. On a constant-time match, the auth middleware authorizes a full-access platform operator — the whole data plane and the/v1/admin/*management surface — without minting a JWT and independently of the JWT verifier.It stamps two things into the request context: the live
admin_role(so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a new platform-operator bit thatRequireAdminhonors even when the policy isnil/deleted — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged.auth.Middlewaremoves to constructor injection —Middleware(cfg, *policy.Store, *slog.Logger)returning the plainfunc(http.Handler) http.Handler— which keeps the existing test call sites unchanged. The key is compared withcrypto/subtle.ConstantTimeCompare, checked before the Bearer token, read only from a header (never?token=), and its use is audit-logged at Info.Out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions.
Test plan
make cigreen locally (build, lint, unit, integration, e2e, coverage)admin_role; bothRequireAdminbypass pathsWH_AUTH_OPERATOR_KEYenv overrideX-Operator-Keyreaches/v1/admin/policy,/v1/query, and pipe execution; then delete the policy and confirmPUT /v1/admin/policystill succeeds with the key (break-glass), while a normal admin JWT is deniedRelated Issues
Closes #240. Advances the auth-hardening epic #228 and the management/data-plane split #359.
🤖 Generated with Claude Code
https://claude.ai/code/session_01EvtHeugFdjphAePzQawjYE