diff --git a/CHANGELOG.md b/CHANGELOG.md index 74c90764..625aef79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Public (no-token) read access** via a usable `default_role`: setting it opens public access, removing it closes it, live on a policy PUT/delete. Setting `default_role` equal to the `admin_role` is permitted and makes every roleless request admin (including `/v1/admin/*`) — a local/dev-only convenience that is not refused by `ResolveRole` or `Validate`, but the store logs a loud warning on every node that adopts such a policy (`policy.DefaultRoleGrantsAdmin`). Roles do **not** inherit the default's permissions. - **Schema discovery and DLQ stats are now admin-only** (`RequireAdmin`), replacing the prior token-only `RequireAuthenticated` gate (removed). The `"*"` any-role wildcard remains removed from `policy.Evaluate` and pipe allowlists (column allow-lists unaffected). - **Fail-closed by structure, not a flag.** `Evaluate(nil)`/`IsAdmin(nil)` deny everyone — including the admin role — so deleting the policy from KV is a total lockout on its own (a fresh or emptied deployment is bootstrapped from the policy file, not an implicit `admin` grant); the `Store.SetFailClosed` retention hack and the `allowAnon` admit-decision plumbing are removed. + - **Every authorization denial emits a structured `slog` WARN** (`internal/api/errors.go`). One shared `writeAuthzDenied` path logs `reason`, `role_observed` vs `role_resolved`, `roles_allowed`, the chi `route` pattern (low-cardinality, no concrete path params), `method`, `status`, and a `gate` tag that names the check that denied — `admin` (the `RequireAdmin` gate, including `/v1/schema` and `/v1/dlq/stats`, which carry no `/admin` prefix), `policy` (the per-table evaluator, which also logs the `table` + `action`), or `pipe` (which also logs the pipe name). The denial gates (`RequireAdmin` and the ingest/structured-query/pipes handlers) take an injected `*slog.Logger` (wired from `main`, like the policy/pipes stores) rather than reaching for `slog.Default()`, and log with the request context — so the record inherits the process logger's format/destination and, under the OTel logger, its `trace_id`/`span_id`, making a misconfigured role or policy visible without reproducing the request. ### Changed @@ -70,7 +71,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security -- **BREAKING: raw SQL moves from `POST /v1/query` to `POST /v1/admin/query` and is gated on the admin role** (`internal/api/router.go`, `internal/api/router_test.go`, `internal/api/query.go`, `internal/api/query_test.go`, `internal/policy/policy.go`, `internal/policy/policy_test.go`, `cmd/wavehouse/main.go`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/getting-started.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/why-wavehouse.md`, `clients/ts/src/sql.ts`, `clients/ts/src/types.ts`, `clients/ts/src/client.test.ts`, `clients/ts/src/namespaces.test.ts`, `tests/integration/query_test.go`, `tests/e2e/sdk/admin.test.ts`, `tests/e2e/sdk/setup.ts`, `tests/e2e/sdk/query.test.ts`, `internal/ingest/bento.go`, `README.md`, `SECURITY.md`, `AGENTS.md`, `.gemini/styleguide.md`, `.github/prompts/pr-review.md`). Previously, raw SQL was mounted at `/v1/query` under the `/v1` auth middleware with the authorization decision deferred to the handler — any caller with a `policy.RolePermissions.raw_sql: true` grant on any table could submit raw SQL and slip through as a non-admin. The route now lives under the `/v1/admin/*` group, so its URL telegraphs the access level, and it shares the surrounding `RequireAdmin` gate with the rest of that tree (policy CRUD, pipes CRUD, log-level). Raw SQL has no per-statement scope check (we cannot authorize predicates without a full SQL parser), so the admin gate is the entire authorization story; a separate, tighter gate just for raw SQL would be redundant, since the whole `/v1/admin/*` tree already requires the admin role. The `policy.RolePermissions.raw_sql` (`RawSQL`) field is **removed** outright from `Policy`/`ResolvedPermissions` and from the `Evaluate` return value — operators with `raw_sql: true` in `policy.yaml` will see a YAML-load warning for the unknown field but the field is otherwise ignored; the equivalent capability is now expressed only as "issue the JWT with the admin role." The in-handler `PolicyStore` plumbing and the `if h.PolicyStore != nil { … }` raw-SQL check inside `QueryHandler.Handle` are deleted along with the field. A new router-level test (`TestNewRouter_RawSQLAdminGate`) pins the contract: admin reaches the handler, while service, viewer, and no-role requests are all 403 (`service` is no longer privileged). The four obsolete handler-level policy tests (`TestQueryHandler_Policy*`, `TestQueryHandler_NoPolicyAllowsAll`) are removed. The shared `/v1/admin/*` gate is declared once as a `requireAdmin` (`RequireAdmin`) local at the top of the `/v1` route closure. The normal surfaces for non-admin callers — `POST /v1/ingest?table={table}`, `POST /v1/query?table={table}`, `GET/POST /v1/pipes/{name}` — are unchanged. +- **BREAKING: raw SQL moves from `POST /v1/query` to `POST /v1/admin/query` and is gated on the admin role** (`internal/api/router.go`, `internal/api/router_test.go`, `internal/api/query.go`, `internal/api/query_test.go`, `internal/policy/policy.go`, `internal/policy/policy_test.go`, `cmd/wavehouse/main.go`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/getting-started.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/why-wavehouse.md`, `clients/ts/src/sql.ts`, `clients/ts/src/types.ts`, `clients/ts/src/client.test.ts`, `clients/ts/src/namespaces.test.ts`, `tests/integration/query_test.go`, `tests/e2e/sdk/admin.test.ts`, `tests/e2e/sdk/setup.ts`, `tests/e2e/sdk/query.test.ts`, `internal/ingest/bento.go`, `README.md`, `SECURITY.md`, `AGENTS.md`, `.gemini/styleguide.md`, `.github/prompts/pr-review.md`). Previously, raw SQL was mounted at `/v1/query` under the `/v1` auth middleware with the authorization decision deferred to the handler — any caller with a `policy.RolePermissions.raw_sql: true` grant on any table could submit raw SQL and slip through as a non-admin. The route now lives under the `/v1/admin/*` group, so its URL telegraphs the access level, and it shares the surrounding `RequireAdmin` gate with the rest of that tree (policy CRUD, pipes CRUD). Raw SQL has no per-statement scope check (we cannot authorize predicates without a full SQL parser), so the admin gate is the entire authorization story; a separate, tighter gate just for raw SQL would be redundant, since the whole `/v1/admin/*` tree already requires the admin role. The `policy.RolePermissions.raw_sql` (`RawSQL`) field is **removed** outright from `Policy`/`ResolvedPermissions` and from the `Evaluate` return value — operators with `raw_sql: true` in `policy.yaml` will see a YAML-load warning for the unknown field but the field is otherwise ignored; the equivalent capability is now expressed only as "issue the JWT with the admin role." The in-handler `PolicyStore` plumbing and the `if h.PolicyStore != nil { … }` raw-SQL check inside `QueryHandler.Handle` are deleted along with the field. A new router-level test (`TestNewRouter_RawSQLAdminGate`) pins the contract: admin reaches the handler, while service, viewer, and no-role requests are all 403 (`service` is no longer privileged). The four obsolete handler-level policy tests (`TestQueryHandler_Policy*`, `TestQueryHandler_NoPolicyAllowsAll`) are removed. The shared `/v1/admin/*` gate is declared once as a `requireAdmin` (`RequireAdmin`) local at the top of the `/v1` route closure. The normal surfaces for non-admin callers — `POST /v1/ingest?table={table}`, `POST /v1/query?table={table}`, `GET/POST /v1/pipes/{name}` — are unchanged. - **Per-pipe `allowed_roles` now fails closed on an empty role** (`internal/api/pipes.go`, `internal/api/pipes_test.go`, `docs/src/content/docs/api.md`): `PipesHandler.Execute` enforced a pipe's `allowed_roles` allowlist only when the request carried a non-empty role, so an empty or absent role skipped the check and the restricted pipe was served. Per-pipe `allowed_roles` is the only authorization gate on the execute path (`GET/POST /v1/pipes/{name}` sit outside the `/v1/admin/*` `RequireAdmin` gate), so any roleless request reached a restricted pipe unchecked — triggered whenever a request carried no token, or a token missing the configured `auth.role_claim` (either way the resolved role is empty). Removed the `if role != ""` guard so an empty role flows into the scan, matches nothing, and returns `403`; empty allowlist entries are skipped so a stray `""` can't authorize an empty role. The gap that hid this — every prior role test set a non-empty role — is closed by consolidating the four standalone role tests into a table-driven `TestPipesHandler_Execute_RoleAuthorization` matrix that pins the empty/absent-role rows, plus a focused `TestPipesHandler_Execute_RestrictedPipe_EmptyRoleDenied` regression. Closes #159. - **Access-control policies reject empty role names and never match a roleless request to an empty role key** (`internal/policy/policy.go`, `internal/policy/policy_test.go`): the #159 step-3 audit (cross-check `internal/policy/` against empty roles) surfaced the policy-side twin of the empty-`allowed_roles`-entry footgun. `Evaluate` did a direct `rolePerms[role]` lookup, so a stray `""` role key in `policy.yaml` would have authorized any request whose resolved role is empty (no token, or a JWT missing `auth.role_claim`) on the policy-gated paths (`POST /v1/query?table={table}`, `POST /v1/ingest?table={table}`, the SSE/WS streams). Two complementary guards: `Validate` now rejects empty/whitespace role keys at write/bootstrap time, and `Evaluate` skips the direct key lookup for an empty role so a roleless request can only be authorized by the configured `admin_role` (or a `default_role` that resolves to a real listed role) — fail-closed even if a malformed policy reaches the engine from KV. Regression tests pin both guards. Part of #159. - **`minimumReleaseAge: 10080` (7 days) on every pnpm workspace** (`clients/ts/pnpm-workspace.yaml`, `docs/pnpm-workspace.yaml`, `tests/e2e/sdk/pnpm-workspace.yaml`): pnpm 11 will refuse to install any package published in the last seven days, giving npm and security researchers time to flag a compromised release before it lands in our lockfile. Existing locked versions are grandfathered. For a one-off override on an urgent hotfix release, list the package under `minimumReleaseAgeExclude:` in the same file. Part of #160. @@ -176,7 +177,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **All three `cmd/` binaries use a `run() int` pattern** (`cmd/wavehouse`, `cmd/wavehouse-api`, `cmd/wavehouse-worker`, `tests/cmd/bento_pub`): `main()` now calls `os.Exit(run())` so deferred OTEL shutdown and resource `Close()` calls actually fire on error exit — previously every `os.Exit(1)` inside `main()` silently skipped them. All HTTP servers gained a `ReadHeaderTimeout: 10s` to close gosec G112 (Slowloris). - **Errcheck sweep across `cmd/` and `internal/`**: wrapped deferred `Close` / `Shutdown` calls in `func() { _ = x.Close() }()` and acknowledged ignored errors on `json.Encoder.Encode`, `http.ResponseWriter.Write`, `fmt.Fprintf`, `rows.Close`, and NATS `m.Ack` / `m.Nak`. `jsInput.Read`'s Ack/Nak paths now log a warning on failure so lost acks are visible. -- **Log-level PUT response uses `json.Encoder`** (`internal/api/router.go`): the old raw string-concat `Write` of a user-supplied path value tripped gosec `G705` (XSS via taint). Echoes `newLevel.String()` (the parsed level) rather than the raw request value, closing the taint flow. - **Narrowed `revive` ruleset in `.golangci.yml`** to an explicit list of semantically useful rules (`error-return`, `context-as-argument`, `var-naming`, etc.). Dropped the defaults `exported`, `package-comments`, and `unused-parameter` which together produced ~50 findings that were pure comment-style noise for internal-only packages. Easy to re-enable per-rule later if the team wants to require doc comments. - **Unit test coverage threshold temporarily lowered 70% → 60%** in `.github/workflows/ci.yml` and `Makefile`, and added `.testcoverage.yml` with `exclude.paths` for packages that have no `_test.go` files (`internal/dedupe`, `internal/mq`, `internal/observability`, `internal/testutil`). Those packages get linked into test binaries and so appeared in the coverage profile at 0%, dragging the measured total to 54.7% even though the tested surface was at 64.9%. After the exclusions, measured total is **64.9%** vs a 60% threshold. Restoring the 70% target + deleting the exclusions is tracked in **#67**. - **Debug `fmt.Println` / `fmt.Printf` removed from `cmd/*/main.go`** — replaced with `logger.Info` / `logger.Error` lines or folded into the existing startup log. Prevented unformatted plaintext lines from appearing interleaved with JSON logs in production (flagged by Claude's re-review of #66). Also dropped the hardcoded `172.18.240.1:4317` WSL-gateway default in `cmd/wavehouse-worker/main.go` — now defaults to `127.0.0.1:4317` like the other two binaries. diff --git a/cmd/wavehouse/main.go b/cmd/wavehouse/main.go index c3f1c79e..ed078105 100644 --- a/cmd/wavehouse/main.go +++ b/cmd/wavehouse/main.go @@ -319,7 +319,7 @@ func run() int { // Build handlers. js := embeddedMQ.JetStream() - ingestHandler := api.NewIngestHandler(registry, embeddedMQ) + ingestHandler := api.NewIngestHandler(registry, embeddedMQ, logger) ingestHandler.PolicyStore = policyStore if dedup != nil { ingestHandler.Dedup = dedup @@ -376,13 +376,13 @@ func run() int { Schema: api.NewSchemaHandler(registry), DLQ: dlqHandler, Policy: api.NewPolicyHandler(policyStore), - Pipes: api.NewPipesHandler(pipesStore, policyStore, chConn, cache, cfg.ClickHouse.QueryTimeout), - StructuredQuery: api.NewStructuredQueryHandler(chConn, cache, registry, policyStore, cfg.Cache.TimestampBucketSeconds, cfg.ClickHouse.QueryTimeout), + Pipes: api.NewPipesHandler(pipesStore, policyStore, chConn, cache, cfg.ClickHouse.QueryTimeout, logger), + StructuredQuery: api.NewStructuredQueryHandler(chConn, cache, registry, policyStore, cfg.Cache.TimestampBucketSeconds, cfg.ClickHouse.QueryTimeout, logger), AuthMW: authMW, PolicyStore: policyStore, + Logger: logger, JS: js, CORSOrigins: cfg.Server.CORSAllowedOrigins, - LogLevel: logLevel, } // Prometheus /metrics routing: same-port → mount on API router, diff --git a/docs/src/content/docs/api.md b/docs/src/content/docs/api.md index db4b88c6..e1cb9541 100644 --- a/docs/src/content/docs/api.md +++ b/docs/src/content/docs/api.md @@ -191,7 +191,7 @@ Executes a SQL statement directly against ClickHouse. **WaveHouse proxies the SQ This endpoint **does not cache, does not singleflight, and emits `Cache-Control: no-store`** — every request goes straight to ClickHouse, mutation or read, and downstream HTTP caches are explicitly told not to store the response. Raw SQL is an admin escape hatch with infrequent, ad-hoc traffic, so the L1/singleflight machinery would only add complexity without a real hit-rate win. Use [`POST /v1/query?table={table}`](#post-v1querytabletable--structured-query) or [`GET/POST /v1/pipes/{name}`](#getpost-v1pipesname--execute-named-pipe) for the cached read paths (dashboards, high-QPS clients, etc.) — both share an in-process L1 (Ristretto) with singleflight coalescing. -> **Admin only.** The route is mounted under `/v1/admin/*`, behind the `RequireAdmin` gate: only a caller whose JWT role equals the policy `admin_role` (`"admin"` by default) may use it. A request with no/invalid token resolves to the `default_role` (not the admin role unless `default_role` is deliberately set to it — a loudly-warned dev-only setting) and is rejected. Raw SQL has no per-statement scope check (a full SQL parser would be needed to authorize predicates), so the role gate is the entire authorization story, shared with the rest of `/v1/admin/*` (policy CRUD, pipes CRUD, log-level). The normal surfaces for non-admin callers are `POST /v1/ingest?table={table}` for writes, `POST /v1/query?table={table}` for structured reads, and `GET/POST /v1/pipes/{name}` for pre-defined queries — none of which expose raw SQL. +> **Admin only.** The route is mounted under `/v1/admin/*`, behind the `RequireAdmin` gate: only a caller whose JWT role equals the policy `admin_role` (`"admin"` by default) may use it. A request with no/invalid token resolves to the `default_role` (not the admin role unless `default_role` is deliberately set to it — a loudly-warned dev-only setting) and is rejected. Raw SQL has no per-statement scope check (a full SQL parser would be needed to authorize predicates), so the role gate is the entire authorization story, shared with the rest of `/v1/admin/*` (policy CRUD, pipes CRUD). The normal surfaces for non-admin callers are `POST /v1/ingest?table={table}` for writes, `POST /v1/query?table={table}` for structured reads, and `GET/POST /v1/pipes/{name}` for pre-defined queries — none of which expose raw SQL. `/v1/admin/query` is the only sanctioned surface for non-insert mutations (the ingest pipeline is insert-only). Granting raw-SQL access to a non-admin role via the policy engine is no longer supported: authenticate with the admin role (`admin_role`). diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index 1b79d82e..7054bad8 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -68,7 +68,7 @@ internal/ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with standard middleware (RequestID, RealIP, Recoverer). -- **router.go** — Route definitions. Public: `/health`, `/ready`. Policy-gated: `/v1/ingest?table={table}`, `/v1/query?table={table}` (structured), `/v1/pipes/{name}` (named pipes), `/v1/stream/sse`, `/v1/stream/ws`. Admin-only (`RequireAdmin`, role == `policy.admin_role`): `/v1/schema/*`, `/v1/dlq/stats`, `/v1/admin/policy`, `/v1/admin/pipes/*`, `/v1/admin/log-level`, `/v1/admin/query` (raw SQL — same gate as the rest of `/v1/admin/*`). +- **router.go** — Route definitions. Public: `/health`, `/ready`. Policy-gated: `/v1/ingest?table={table}`, `/v1/query?table={table}` (structured), `/v1/pipes/{name}` (named pipes), `/v1/stream/sse`, `/v1/stream/ws`. Admin-only (`RequireAdmin`, role == `policy.admin_role`): `/v1/schema/*`, `/v1/dlq/stats`, `/v1/admin/policy`, `/v1/admin/pipes/*`, `/v1/admin/query` (raw SQL — same gate as the rest of `/v1/admin/*`). - **`internal/auth`** — JWT auth middleware supporting HMAC and JWKS validation and role extraction from a configurable claim path. It always runs (no on/off flag) and never rejects: a missing/invalid/expired token yields an empty role (resolved to `default_role` downstream), with the token error stashed in context so a denying gate can fail loud. - **policy.go** — CRUD handler for access control policies (`/v1/admin/policy`). - **pipes.go** — Named query pipe handlers: admin CRUD and execution with parameter binding. @@ -179,7 +179,7 @@ Active Sweeper (async goroutine, every 60s): Client POST /v1/admin/query → JWT auth middleware (always runs; no/invalid token → empty role) → /v1/admin RequireAdmin (role == policy.admin_role) — single gate shared - with the rest of /v1/admin/* (policy CRUD, pipes CRUD, log-level). Raw SQL has + with the rest of /v1/admin/* (policy CRUD, pipes CRUD). Raw SQL has no per-statement scope check (a full SQL parser would be needed to authorize predicates), so the role gate is the entire authorization story. /v1/admin/query is the only sanctioned surface for non-SELECT diff --git a/internal/api/errors.go b/internal/api/errors.go index ef01c85b..ec83363e 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -2,9 +2,11 @@ package api import ( "encoding/json" + "log/slog" "net/http" "github.com/Wave-RF/WaveHouse/internal/auth" + "github.com/go-chi/chi/v5" ) // writeJSONError writes a JSON error response with the correct Content-Type @@ -22,23 +24,88 @@ func writeJSONError(w http.ResponseWriter, status int, message string) { _ = json.NewEncoder(w).Encode(map[string]string{"error": message}) } -// writeAuthzDenied writes the response for an authorization denial. When the -// request carried a present-but-invalid token (recorded by the auth -// middleware), it fails loud — 401 with the token reason ("token expired" / -// "invalid token") — so a caller whose bad token silently fell back to the -// default role learns why it lacks access, instead of a bare "forbidden". -// Otherwise it's an ordinary 403 for the caller's resolved role. +// writeAuthzDenied writes the response for an authorization denial and emits a +// structured WARN (see logAuthzDenied) so a misconfigured role or policy shows +// up in the logs without having to reproduce the request. When the request +// carried a present-but-invalid token (recorded by the auth middleware), it +// fails loud — 401 with the token reason ("token expired" / "invalid token") — +// so a caller whose bad token silently fell back to the default role learns why +// it lacks access, instead of a bare "forbidden". Otherwise it's an ordinary +// 403 for the caller's resolved role. // // Pass the role AFTER default-role resolution so forbiddenForRole's empty-role -// message is accurate. -func writeAuthzDenied(w http.ResponseWriter, r *http.Request, role string) { - if err := auth.AuthErrorFromContext(r.Context()); err != nil { - writeJSONError(w, http.StatusUnauthorized, err.Error()) +// message is accurate. allowedRoles is the set the gate would have accepted (a +// pipe's allowed_roles); the gates with no flat role list — the /v1/admin gate +// and the policy-evaluator paths (ingest, structured query) — pass nil. attrs +// are gate-specific structured fields appended to the WARN: each gate tags a +// "gate" (admin / policy / pipe) so a denial is attributable to the check that +// raised it (the route pattern alone can't — /v1/schema runs the admin gate, +// not a policy one), and the policy paths add the table + action they evaluated. +// logger is the calling gate's injected logger (each handler holds one; main +// wires it, tests pass their own) — the denial WARN goes there, not to a +// package global. +func writeAuthzDenied(w http.ResponseWriter, r *http.Request, logger *slog.Logger, role string, allowedRoles []string, attrs ...slog.Attr) { + authErr := auth.AuthErrorFromContext(r.Context()) + + // reason tracks the response: a present-but-invalid token fails loud (401) + // with the sanitized token reason; otherwise it's a 403, split so the + // empty-role case (no token / no role claim AND no default_role) is greppable + // apart from a concrete role that simply isn't on the allowlist. + status, reason := http.StatusForbidden, "role not in allowed roles" + switch { + case authErr != nil: + status, reason = http.StatusUnauthorized, authErr.Error() + case role == "": + reason = "no role and no default_role configured" + } + + logAuthzDenied(logger, r, reason, role, allowedRoles, status, attrs...) + + if authErr != nil { + writeJSONError(w, http.StatusUnauthorized, authErr.Error()) return } writeJSONError(w, http.StatusForbidden, forbiddenForRole(role)) } +// logAuthzDenied emits the structured WARN for an authorization denial so +// operators see a misconfigured role or policy immediately: reason, the +// observed (pre-default-resolution) and resolved roles, the roles the gate +// would have accepted, and the matched route + method. role_observed empty with +// a non-empty role_resolved means the caller presented no role and was mapped to +// default_role; roles_allowed is populated only by the pipe gate (a pipe's +// allowed_roles) and is empty for the /v1/admin gate and the policy-evaluator +// paths (ingest, structured query). attrs carry each gate's own fields (the +// "gate" tag, plus table + action on the policy paths) so the records stay +// distinguishable beyond the route. +// +// slog escapes control characters in string values, so the request-derived +// fields (route, method, role) carry no log-injection risk despite originating +// in an *http.Request scope. +func logAuthzDenied(logger *slog.Logger, r *http.Request, reason, resolvedRole string, allowedRoles []string, status int, attrs ...slog.Attr) { + // Prefer the matched route template (e.g. /v1/pipes/{name}) over the raw + // path: it keeps the field low-cardinality and avoids logging concrete path + // params. Falls back to the path when there's no chi route context (a gate + // exercised outside the router, e.g. in a unit test). + route := r.URL.Path + if rctx := chi.RouteContext(r.Context()); rctx != nil { + if pattern := rctx.RoutePattern(); pattern != "" { + route = pattern + } + } + fields := []slog.Attr{ + slog.String("reason", reason), + slog.String("role_observed", auth.RoleFromContext(r.Context())), + slog.String("role_resolved", resolvedRole), + slog.Any("roles_allowed", allowedRoles), + slog.String("route", route), + slog.String("method", r.Method), + slog.Int("status", status), + } + + logger.LogAttrs(r.Context(), slog.LevelWarn, "authorization denied", append(fields, attrs...)...) +} + // forbiddenForRole returns the 403 message body for a policy/allowlist denial. // role is the caller's effective (default-resolved) role: an empty role means // the request had no token or no role claim AND no usable default_role, so it diff --git a/internal/api/errors_test.go b/internal/api/errors_test.go index 3dce766a..caf8ddd3 100644 --- a/internal/api/errors_test.go +++ b/internal/api/errors_test.go @@ -1,11 +1,20 @@ package api import ( + "bytes" + "context" "encoding/json" + "errors" + "log/slog" "net/http" "net/http/httptest" "testing" + "github.com/Wave-RF/WaveHouse/internal/auth" + "github.com/Wave-RF/WaveHouse/internal/discovery" + "github.com/Wave-RF/WaveHouse/internal/pipes" + "github.com/Wave-RF/WaveHouse/internal/policy" + "github.com/Wave-RF/WaveHouse/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -33,3 +42,173 @@ func TestWriteJSONError_EscapesSpecialCharacters(t *testing.T) { require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) assert.Equal(t, `oops "quoted" \n`, body["error"]) } + +// warnBufLogger returns a WARN-level JSON logger that writes to the returned +// buffer. It's injected into the gate/handler under test (the way +// internal/policy/store_test.go injects one into NewStore), so reading a +// denial's structured WARN needs no process-global slog.SetDefault swap — which +// is why, unlike the old default-logger capture, these tests can run in parallel. +func warnBufLogger() (*slog.Logger, *bytes.Buffer) { + var buf bytes.Buffer + return slog.New(slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn})), &buf +} + +// TestRequireAdmin_DenialLogsStructuredWarn pins the structured WARN emitted on +// an admin-gate denial: a concrete non-admin role, the route, the "not on the +// allowlist" reason, and gate=admin (so the denial is attributable to the admin +// check, which the route pattern alone can't convey). +func TestRequireAdmin_DenialLogsStructuredWarn(t *testing.T) { + t.Parallel() + logger, buf := warnBufLogger() + handler := RequireAdmin(policy.NewMemoryStore(&policy.Policy{}), logger)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("handler must not run on a denied request") + })) + + ctx := auth.WithRole(context.Background(), "viewer") + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/v1/admin/query", nil) + handler.ServeHTTP(httptest.NewRecorder(), req) + + out := buf.String() + assert.Contains(t, out, `"level":"WARN"`, "a denial that logs nothing is the bug this guards against") + assert.Contains(t, out, `"msg":"authorization denied"`) + assert.Contains(t, out, `"reason":"role not in allowed roles"`) + assert.Contains(t, out, `"role_observed":"viewer"`) + assert.Contains(t, out, `"role_resolved":"viewer"`) + assert.Contains(t, out, `"roles_allowed":null`, "the admin gate logs no explicit allowlist") + assert.Contains(t, out, `"route":"/v1/admin/query"`) + assert.Contains(t, out, `"method":"GET"`) + assert.Contains(t, out, `"status":403`) + assert.Contains(t, out, `"gate":"admin"`) +} + +// TestRequireAdmin_EmptyRoleDenialLogsResolvedRole: a tokenless request maps to +// the policy default_role before the admin check, so role_observed is empty +// while role_resolved is the default — the signal that says "the public default +// role can't reach admin", not "the client sent the wrong role". +func TestRequireAdmin_EmptyRoleDenialLogsResolvedRole(t *testing.T) { + t.Parallel() + logger, buf := warnBufLogger() + store := policy.NewMemoryStore(&policy.Policy{DefaultRole: "viewer"}) + handler := RequireAdmin(store, logger)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("handler must not run on a denied request") + })) + + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/v1/admin/query", nil) + handler.ServeHTTP(httptest.NewRecorder(), req) + + out := buf.String() + assert.Contains(t, out, `"msg":"authorization denied"`) + assert.Contains(t, out, `"role_observed":""`, "no token / role claim was presented") + assert.Contains(t, out, `"role_resolved":"viewer"`, "empty role resolved to default_role") + assert.Contains(t, out, `"reason":"role not in allowed roles"`) +} + +// TestRequireAdmin_InvalidTokenDenialLogsFailLoudReason: a present-but-invalid +// token fails loud — the WARN reason carries the token error and the status is +// 401, distinguishing it from an ordinary roleless 403. The admin gate logs no +// explicit allowlist, so roles_allowed is null. +func TestRequireAdmin_InvalidTokenDenialLogsFailLoudReason(t *testing.T) { + t.Parallel() + logger, buf := warnBufLogger() + handler := RequireAdmin(nil, logger)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("handler must not run on a denied request") + })) + + ctx := auth.WithAuthError(context.Background(), errors.New("token expired")) + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/v1/admin/query", nil) + handler.ServeHTTP(httptest.NewRecorder(), req) + + out := buf.String() + assert.Contains(t, out, `"msg":"authorization denied"`) + assert.Contains(t, out, `"reason":"token expired"`) + assert.Contains(t, out, `"status":401`) + assert.Contains(t, out, `"roles_allowed":null`) +} + +// TestPipesHandler_Execute_DenialLogsAllowedRoles: a pipe denial logs the pipe's +// allowed_roles as roles_allowed and gate=pipe with the pipe name, so an +// operator who forgot to grant a role sees the exact set that would have let the +// caller through — and which pipe (the route pattern is /v1/pipes/{name}, so the +// concrete name isn't in the route field). +func TestPipesHandler_Execute_DenialLogsAllowedRoles(t *testing.T) { + t.Parallel() + logger, buf := warnBufLogger() + store := pipes.NewMemoryStore( + &pipes.NamedQuery{Name: "report", SQL: "SELECT * FROM clicks", AllowedRoles: []string{"analyst", "viewer"}}, + ) + h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0, logger) + + r := pipesRequest(t, http.MethodPost, "/v1/pipes/report/execute", "report", nil) + r = r.WithContext(auth.WithRole(r.Context(), "guest")) + w := httptest.NewRecorder() + h.Execute(w, r) + require.Equal(t, http.StatusForbidden, w.Code) + + out := buf.String() + assert.Contains(t, out, `"msg":"authorization denied"`) + assert.Contains(t, out, `"role_observed":"guest"`) + assert.Contains(t, out, `"roles_allowed":["analyst","viewer"]`) + assert.Contains(t, out, `"reason":"role not in allowed roles"`) + assert.Contains(t, out, `"method":"POST"`) + assert.Contains(t, out, `"gate":"pipe"`) + assert.Contains(t, out, `"pipe":"report"`) +} + +// TestIngest_DenialLogsPolicyGate: the policy-evaluator denial carries +// gate=policy plus the table and action it evaluated, so a per-table policy +// denial is distinguishable from an admin-gate or pipe-allowlist denial — the +// /v1/ingest route pattern alone doesn't say which check failed, or on what. +func TestIngest_DenialLogsPolicyGate(t *testing.T) { + t.Parallel() + logger, buf := warnBufLogger() + h := NewIngestHandler(testRegistry(), &testutil.MockPublisher{}, logger) + h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ + Tables: map[string]policy.TablePolicy{ + "clicks": {Select: map[string]policy.RolePermissions{"viewer": {}}}, // no insert for viewer + }, + }) + + req := ingestRequest(t, "clicks", map[string]any{"page": "/home"}) + req = req.WithContext(auth.WithRole(req.Context(), "viewer")) + w := httptest.NewRecorder() + h.Handle(w, req) + require.Equal(t, http.StatusForbidden, w.Code) + + out := buf.String() + assert.Contains(t, out, `"msg":"authorization denied"`) + assert.Contains(t, out, `"gate":"policy"`) + assert.Contains(t, out, `"table":"clicks"`) + assert.Contains(t, out, `"action":"insert"`) + assert.Contains(t, out, `"role_resolved":"viewer"`) +} + +// TestAuthzDenied_LogsChiRoutePattern: routed through the real mux, the WARN's +// route is the matched route template, not the raw path — low-cardinality and +// free of concrete path params. /v1/schema runs the admin gate (no /admin +// prefix), so gate=admin is what tells the operator which check denied it. +func TestAuthzDenied_LogsChiRoutePattern(t *testing.T) { + t.Parallel() + logger, buf := warnBufLogger() + reg := discovery.NewSchemaRegistryFromMap(nil) + router := NewRouter(Dependencies{ + Ingest: NewIngestHandler(reg, &testutil.MockPublisher{}, logger), + Query: &QueryHandler{}, + SSE: NewSSEHandler(NewHub(), nil), + WS: NewWSHandler(NewHub(), nil, nil), + Health: &HealthHandler{}, + Schema: NewSchemaHandler(reg), + AuthMW: func(next http.Handler) http.Handler { return next }, + PolicyStore: policy.NewMemoryStore(&policy.Policy{}), + Logger: logger, + }) + + ctx := auth.WithRole(context.Background(), "viewer") + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/v1/schema", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + require.Equal(t, http.StatusForbidden, rec.Code) + + out := buf.String() + assert.Contains(t, out, `"route":"/v1/schema"`, "route should be the chi pattern") + assert.Contains(t, out, `"gate":"admin"`, "/v1/schema runs the admin gate, not a policy one") +} diff --git a/internal/api/ingest.go b/internal/api/ingest.go index a1c2f9c1..c9d628a1 100644 --- a/internal/api/ingest.go +++ b/internal/api/ingest.go @@ -28,10 +28,11 @@ type IngestHandler struct { IDField string // dedup key field name (e.g. "event_id") Publisher mq.Publisher PolicyStore *policy.Store + logger *slog.Logger } -func NewIngestHandler(registry *discovery.SchemaRegistry, pub mq.Publisher) *IngestHandler { - return &IngestHandler{Registry: registry, Publisher: pub} +func NewIngestHandler(registry *discovery.SchemaRegistry, pub mq.Publisher, logger *slog.Logger) *IngestHandler { + return &IngestHandler{Registry: registry, Publisher: pub, logger: logger} } func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { @@ -47,14 +48,14 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { defer span.End() // Add a standard log to prove we are inside the span logic - slog.DebugContext(ctx, "debug: span started for ingest", "table", table) + h.logger.DebugContext(ctx, "debug: span started for ingest", "table", table) r = r.WithContext(ctx) // TODO: what should the order of these be to maximize speed + limit risk of data leakage or DoS/resource exhaustion? if table == "" { - slog.ErrorContext(ctx, "missing table parameter in request") + h.logger.ErrorContext(ctx, "missing table parameter in request") writeJSONError(w, http.StatusBadRequest, "missing table") return } @@ -62,7 +63,7 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { // TODO: prevent table-enumeration... schema := h.Registry.Get(table) if schema == nil { - slog.WarnContext(ctx, "unknown table requested", "table", table) + h.logger.WarnContext(ctx, "unknown table requested", "table", table) writeJSONError(w, http.StatusNotFound, "unknown table: "+table) return } @@ -77,8 +78,11 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { claims, _ := auth.ClaimsFromContext(ctx) perms = policy.Evaluate(p, role, table, "insert", claims) if !perms.Allowed { - slog.WarnContext(ctx, "policy enforcement rejected request", "role", role, "table", table) - writeAuthzDenied(w, r, role) + writeAuthzDenied(w, r, h.logger, role, nil, + slog.String("gate", "policy"), + slog.String("table", table), + slog.String("action", "insert"), + ) return } } @@ -88,13 +92,13 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { decoder := json.NewDecoder(r.Body) decoder.UseNumber() if err := decoder.Decode(&data); err != nil { - slog.ErrorContext(ctx, "invalid json payload", "error", err, "table", table) + h.logger.ErrorContext(ctx, "invalid json payload", "error", err, "table", table) writeJSONError(w, http.StatusBadRequest, "invalid json") return } if err := discovery.Validate(schema, data); err != nil { - slog.WarnContext(ctx, "schema validation failed", "error", err, "table", table) + h.logger.WarnContext(ctx, "schema validation failed", "error", err, "table", table) writeJSONError(w, http.StatusBadRequest, err.Error()) return } @@ -104,7 +108,7 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { // Check column permissions — reject disallowed columns. for col := range data { if !perms.IsColumnAllowed(col) { - slog.WarnContext(ctx, "column insertion forbidden", "column", col, "role", role) + h.logger.WarnContext(ctx, "column insertion forbidden", "column", col, "role", role) writeJSONError(w, http.StatusForbidden, fmt.Sprintf("column %q not allowed for insert", col)) return } @@ -113,7 +117,7 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { for col, requiredVal := range perms.CheckClauses { if actual, ok := data[col]; ok { if fmt.Sprint(actual) != fmt.Sprint(requiredVal) { - slog.WarnContext(ctx, "check clause failed", "column", col, "expected", requiredVal, "actual", actual) + h.logger.WarnContext(ctx, "check clause failed", "column", col, "expected", requiredVal, "actual", actual) writeJSONError(w, http.StatusForbidden, fmt.Sprintf("check failed for column %q", col)) return } @@ -130,12 +134,12 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { eventID := fmt.Sprint(idVal) dup, err := h.Dedup.CheckAndMark(ctx, eventID) if err != nil { - slog.ErrorContext(ctx, "dedupe check failed", "error", err, "event_id", eventID) + h.logger.ErrorContext(ctx, "dedupe check failed", "error", err, "event_id", eventID) writeJSONError(w, http.StatusInternalServerError, "dedupe failed") return } if dup { - slog.InfoContext(ctx, "duplicate event skipped", "event_id", eventID) + h.logger.InfoContext(ctx, "duplicate event skipped", "event_id", eventID) w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]bool{"duplicate": true}) return @@ -155,7 +159,7 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { payload, err := json.Marshal(evt) if err != nil { - slog.ErrorContext(ctx, "failed to marshal event message", "error", err) + h.logger.ErrorContext(ctx, "failed to marshal event message", "error", err) writeJSONError(w, http.StatusInternalServerError, "marshal failed") return } @@ -165,20 +169,20 @@ func (h *IngestHandler) Handle(w http.ResponseWriter, r *http.Request) { subject += "." + query.SafeEncodeNATS(scope) } - slog.DebugContext(ctx, "publishing event to NATS", "subject", subject, "table", table, "scope", scope) + h.logger.DebugContext(ctx, "publishing event to NATS", "subject", subject, "table", table, "scope", scope) if err := h.Publisher.Publish(ctx, subject, payload); err != nil { if strings.Contains(err.Error(), "maximum bytes exceeded") { - slog.WarnContext(ctx, "nats maximum bytes exceeded", "subject", subject) + h.logger.WarnContext(ctx, "nats maximum bytes exceeded", "subject", subject) w.Header().Set("Retry-After", "30") writeJSONError(w, http.StatusServiceUnavailable, "service unavailable") return } - slog.ErrorContext(ctx, "failed to publish to NATS", "error", err, "subject", subject) + h.logger.ErrorContext(ctx, "failed to publish to NATS", "error", err, "subject", subject) writeJSONError(w, http.StatusInternalServerError, "publish failed") return } - slog.InfoContext(ctx, "event successfully ingested", "table", table, "subject", subject) + h.logger.InfoContext(ctx, "event successfully ingested", "table", table, "subject", subject) w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]bool{"ok": true}) } diff --git a/internal/api/ingest_test.go b/internal/api/ingest_test.go index 9106e6c1..801f8e99 100644 --- a/internal/api/ingest_test.go +++ b/internal/api/ingest_test.go @@ -45,7 +45,7 @@ func ingestRequest(t *testing.T, table string, body any) *http.Request { func TestIngest_ValidPayload(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) req := ingestRequest(t, "clicks", map[string]any{"page": "/home", "count": 1}) w := httptest.NewRecorder() @@ -95,7 +95,7 @@ func TestIngest_MissingTable(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) req := httptest.NewRequestWithContext( context.Background(), @@ -118,7 +118,7 @@ func TestIngest_MissingTable(t *testing.T) { func TestIngest_UnknownTable(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) req := ingestRequest(t, "nonexistent", map[string]any{"x": 1}) w := httptest.NewRecorder() @@ -132,7 +132,7 @@ func TestIngest_UnknownTable(t *testing.T) { func TestIngest_InvalidJSON(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) r := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/v1/ingest?table=clicks", bytes.NewReader([]byte("not json"))) @@ -146,7 +146,7 @@ func TestIngest_InvalidJSON(t *testing.T) { func TestIngest_SchemaValidation_UnknownField(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) req := ingestRequest(t, "clicks", map[string]any{"page": "/home", "nonexistent_field": 42}) w := httptest.NewRecorder() @@ -160,7 +160,7 @@ func TestIngest_Dedup_FirstTime(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} dedup := testutil.NewMockDeduplicator() - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) h.Dedup = dedup h.IDField = "event_id" @@ -176,7 +176,7 @@ func TestIngest_Dedup_Duplicate(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} dedup := testutil.NewMockDeduplicator() - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) h.Dedup = dedup h.IDField = "event_id" @@ -202,7 +202,7 @@ func TestIngest_Dedup_Duplicate(t *testing.T) { func TestIngest_PublishError_503(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{Err: errors.New("maximum bytes exceeded")} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) req := ingestRequest(t, "clicks", map[string]any{"page": "/home"}) w := httptest.NewRecorder() @@ -216,7 +216,7 @@ func TestIngest_PublishError_503(t *testing.T) { func TestIngest_PublishError_500(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{Err: errors.New("some other error")} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) req := ingestRequest(t, "clicks", map[string]any{"page": "/home"}) w := httptest.NewRecorder() @@ -230,7 +230,7 @@ func TestIngest_PublishError_500(t *testing.T) { func TestIngest_Policy_Forbidden(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ Tables: map[string]policy.TablePolicy{ "clicks": { @@ -257,7 +257,7 @@ func TestIngest_Policy_Forbidden(t *testing.T) { func TestIngest_Policy_ColumnDenied(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ Tables: map[string]policy.TablePolicy{ "clicks": { @@ -285,7 +285,7 @@ func TestIngest_Policy_ColumnDenied(t *testing.T) { func TestIngest_Policy_CheckClause_Mismatch(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) orgTemplate := "{{ jwt.org_id }}" h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ Tables: map[string]policy.TablePolicy{ @@ -317,7 +317,7 @@ func TestIngest_Policy_CheckClause_Mismatch(t *testing.T) { func TestIngest_Policy_CheckClause_Match(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) orgTemplate := "{{ jwt.org_id }}" h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ Tables: map[string]policy.TablePolicy{ @@ -350,7 +350,7 @@ func TestIngest_Policy_CheckClause_Match(t *testing.T) { func TestIngest_Policy_CheckClause_AutoInject(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) orgTemplate := "{{ jwt.org_id }}" h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ Tables: map[string]policy.TablePolicy{ @@ -387,7 +387,7 @@ func TestIngest_Dedup_MissingIDField(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} dedup := testutil.NewMockDeduplicator() - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) h.Dedup = dedup h.IDField = "event_id" @@ -403,7 +403,7 @@ func TestIngest_Dedup_MissingIDField(t *testing.T) { func TestIngest_Policy_DenyColumns(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ Tables: map[string]policy.TablePolicy{ "clicks": { @@ -430,7 +430,7 @@ func TestIngest_Policy_DenyColumns(t *testing.T) { func TestIngest_AdminRole_NoPolicy(t *testing.T) { t.Parallel() pub := &testutil.MockPublisher{} - h := NewIngestHandler(testRegistry(), pub) + h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger()) h.PolicyStore = policy.NewMemoryStore(&policy.Policy{ Tables: map[string]policy.TablePolicy{ "clicks": {}, diff --git a/internal/api/pipes.go b/internal/api/pipes.go index 09ecc650..4949cd82 100644 --- a/internal/api/pipes.go +++ b/internal/api/pipes.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "log/slog" "net/http" "time" @@ -24,10 +25,11 @@ type PipesHandler struct { Cache cache.Cache sf singleflight.Group maxQueryTimeout time.Duration + logger *slog.Logger } -func NewPipesHandler(store *pipes.Store, policyStore *policy.Store, conn driver.Conn, c cache.Cache, queryTimeout time.Duration) *PipesHandler { - return &PipesHandler{Store: store, PolicyStore: policyStore, CHConn: conn, Cache: c, maxQueryTimeout: queryTimeout} +func NewPipesHandler(store *pipes.Store, policyStore *policy.Store, conn driver.Conn, c cache.Cache, queryTimeout time.Duration, logger *slog.Logger) *PipesHandler { + return &PipesHandler{Store: store, PolicyStore: policyStore, CHConn: conn, Cache: c, maxQueryTimeout: queryTimeout, logger: logger} } // List returns all named queries (admin endpoint). @@ -106,7 +108,10 @@ func (h *PipesHandler) Execute(w http.ResponseWriter, r *http.Request) { } role := policy.ResolveRole(p, auth.RoleFromContext(r.Context())) if !policy.RoleAllowed(p, role, q.AllowedRoles) { - writeAuthzDenied(w, r, role) + writeAuthzDenied(w, r, h.logger, role, q.AllowedRoles, + slog.String("gate", "pipe"), + slog.String("pipe", q.Name), + ) return } diff --git a/internal/api/pipes_test.go b/internal/api/pipes_test.go index 77ee589a..426278b2 100644 --- a/internal/api/pipes_test.go +++ b/internal/api/pipes_test.go @@ -41,7 +41,7 @@ func TestPipesHandler_List(t *testing.T) { &pipes.NamedQuery{Name: "top_pages", SQL: "SELECT page, count(*) FROM clicks GROUP BY page"}, &pipes.NamedQuery{Name: "recent", SQL: "SELECT * FROM clicks ORDER BY ts DESC LIMIT 10"}, ) - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/v1/pipes", nil) @@ -58,7 +58,7 @@ func TestPipesHandler_Get_Found(t *testing.T) { store := pipes.NewMemoryStore( &pipes.NamedQuery{Name: "top_pages", SQL: "SELECT page FROM clicks"}, ) - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodGet, "/v1/pipes/top_pages", "top_pages", nil) @@ -73,7 +73,7 @@ func TestPipesHandler_Get_Found(t *testing.T) { func TestPipesHandler_Get_NotFound(t *testing.T) { t.Parallel() store := pipes.NewMemoryStore() - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodGet, "/v1/pipes/nope", "nope", nil) @@ -87,7 +87,7 @@ func TestPipesHandler_Get_NotFound(t *testing.T) { func TestPipesHandler_List_Empty(t *testing.T) { t.Parallel() store := pipes.NewMemoryStore() - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodGet, "/v1/pipes", "", nil) @@ -103,7 +103,7 @@ func TestPipesHandler_List_Empty(t *testing.T) { func TestPipesHandler_Execute_NotFound(t *testing.T) { t.Parallel() store := pipes.NewMemoryStore() - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodPost, "/v1/pipes/nope/execute", "nope", nil) @@ -127,7 +127,7 @@ func TestPipesHandler_Execute_RoleAuthorization(t *testing.T) { // A real (non-nil) policy so the default admin role ("admin") is defined // and bypasses the allowlist, per the matrix. With a nil policy nobody is // admin (total lockout) — covered separately in internal/policy tests. - h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0) + h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodPost, "/v1/pipes/report/execute", "report", nil) @@ -154,7 +154,7 @@ func TestPipesHandler_Execute_RestrictedPipe_EmptyRoleDenied(t *testing.T) { AllowedRoles: []string{"admin"}, }, ) - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() // No ContextKeyRole set, which simulates no token or a JWT without the role claim. @@ -175,7 +175,7 @@ func TestPipesHandler_Execute_DefaultRoleGrantsAccess(t *testing.T) { store := pipes.NewMemoryStore( &pipes.NamedQuery{Name: "report", SQL: "SELECT * FROM clicks", AllowedRoles: []string{"viewer"}}, ) - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) h.PolicyStore = policy.NewMemoryStore(&policy.Policy{DefaultRole: "viewer"}) w := httptest.NewRecorder() @@ -196,7 +196,7 @@ func TestPipesHandler_Execute_DefaultRoleNotInAllowedRolesDenied(t *testing.T) { store := pipes.NewMemoryStore( &pipes.NamedQuery{Name: "admin_report", SQL: "SELECT * FROM clicks", AllowedRoles: []string{"admin"}}, ) - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) h.PolicyStore = policy.NewMemoryStore(&policy.Policy{DefaultRole: "viewer"}) w := httptest.NewRecorder() @@ -220,7 +220,7 @@ func TestPipesHandler_Execute_MissingParam(t *testing.T) { }, }, ) - h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0) + h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() // No query params or body — missing "page". @@ -245,7 +245,7 @@ func TestPipesHandler_Execute_ParamsFromQuery(t *testing.T) { }, }, ) - h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0) + h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/v1/pipes/by_page/execute?page=/home", nil) @@ -264,7 +264,7 @@ func TestPipesHandler_Execute_ParamsFromQuery(t *testing.T) { func TestPipesHandler_Put_InvalidJSON(t *testing.T) { t.Parallel() store := pipes.NewMemoryStore() - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := httptest.NewRequestWithContext(context.Background(), http.MethodPut, "/v1/pipes/test", bytes.NewReader([]byte(`{bad}`))) @@ -281,7 +281,7 @@ func TestPipesHandler_Put_InvalidJSON(t *testing.T) { func TestPipesHandler_Put_Success(t *testing.T) { t.Parallel() store := pipes.NewMemoryStore() - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodPut, "/v1/pipes/new_pipe", "new_pipe", map[string]any{ @@ -300,7 +300,7 @@ func TestPipesHandler_Put_Success(t *testing.T) { func TestPipesHandler_Put_MissingSQL(t *testing.T) { t.Parallel() store := pipes.NewMemoryStore() - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodPut, "/v1/pipes/bad", "bad", map[string]any{ @@ -317,7 +317,7 @@ func TestPipesHandler_Delete_Success(t *testing.T) { store := pipes.NewMemoryStore( &pipes.NamedQuery{Name: "to_delete", SQL: "SELECT 1"}, ) - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodDelete, "/v1/pipes/to_delete", "to_delete", nil) @@ -339,7 +339,7 @@ func TestPipesHandler_Execute_PostBodyParams(t *testing.T) { }, }, ) - h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0) + h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() body := map[string]any{"page": "/about"} @@ -362,7 +362,7 @@ func TestPipesHandler_Execute_NoAllowedRoles_NonAdminDenied(t *testing.T) { store := pipes.NewMemoryStore( &pipes.NamedQuery{Name: "open", SQL: "SELECT * FROM clicks"}, // no AllowedRoles ) - h := NewPipesHandler(store, nil, nil, nil, 0) + h := NewPipesHandler(store, nil, nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodPost, "/v1/pipes/open/execute", "open", nil) @@ -384,7 +384,7 @@ func TestPipesHandler_Execute_NoAllowedRoles_AdminAllowed(t *testing.T) { store := pipes.NewMemoryStore( &pipes.NamedQuery{Name: "open", SQL: "SELECT * FROM clicks"}, ) - h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0) + h := NewPipesHandler(store, policy.NewMemoryStore(&policy.Policy{}), nil, nil, 0, testutil.NopLogger()) w := httptest.NewRecorder() r := pipesRequest(t, http.MethodPost, "/v1/pipes/open/execute", "open", nil) diff --git a/internal/api/router.go b/internal/api/router.go index 2e8ca1dc..90356395 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "errors" "log/slog" "net/http" @@ -35,7 +34,9 @@ type Dependencies struct { PolicyStore *policy.Store JS jetstream.JetStream // for SSE/WS gap-fill CORSOrigins []string // allowed CORS origins; ["*"] = allow all - LogLevel *slog.LevelVar + // Logger is the request-path logger the denial gates use (RequireAdmin and + // the handlers that call writeAuthzDenied). nil falls back to slog.Default(). + Logger *slog.Logger // MetricsHandler, if non-nil, is mounted at MetricsPath as an unauthenticated // endpoint (Prometheus convention). Wired by main.go from the OTel Prometheus // exporter when observability.metrics.prometheus.enabled is true AND port is 0. @@ -106,7 +107,7 @@ func NewRouter(deps Dependencies) http.Handler { // is policy.AdminRole (configurable via admin_role, "admin" by default), // read live from the policy store so changes apply without a restart. // Declaring it once keeps the gate consistent across the tree. - requireAdmin := RequireAdmin(deps.PolicyStore) + requireAdmin := RequireAdmin(deps.PolicyStore, deps.Logger) r.Post("/ingest", deps.Ingest.Handle) r.Get("/stream/sse", deps.SSE.Handle) @@ -160,24 +161,6 @@ func NewRouter(deps Dependencies) http.Handler { r.Put("/pipes/{name}", deps.Pipes.Put) r.Delete("/pipes/{name}", deps.Pipes.Delete) } - if deps.LogLevel != nil { - r.Put("/log-level", func(w http.ResponseWriter, r *http.Request) { - levelStr := r.URL.Query().Get("level") - - var newLevel slog.Level - if err := newLevel.UnmarshalText([]byte(levelStr)); err != nil { - writeJSONError(w, http.StatusBadRequest, "invalid or missing level (use debug, info, warn, error)") - return - } - - deps.LogLevel.Set(newLevel) - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]string{ - "status": "success", - "level": newLevel.String(), - }) - }) - } }) }) @@ -237,7 +220,7 @@ func jsonRecoverer(next http.Handler) http.Handler { // resolves to an empty (non-admin) role and is denied here. Denials go through // writeAuthzDenied, so a present-but-invalid token fails loud (401 + token // reason) rather than as a bare 403. -func RequireAdmin(store *policy.Store) func(http.Handler) http.Handler { +func RequireAdmin(store *policy.Store, logger *slog.Logger) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var p *policy.Policy @@ -249,7 +232,7 @@ func RequireAdmin(store *policy.Store) func(http.Handler) http.Handler { next.ServeHTTP(w, r) return } - writeAuthzDenied(w, r, role) + writeAuthzDenied(w, r, logger, role, nil, slog.String("gate", "admin")) }) } } diff --git a/internal/api/router_test.go b/internal/api/router_test.go index 84635944..6658c4cf 100644 --- a/internal/api/router_test.go +++ b/internal/api/router_test.go @@ -16,7 +16,7 @@ import ( func TestRequireAdmin_AdminAllowed(t *testing.T) { t.Parallel() - handler := RequireAdmin(policy.NewMemoryStore(&policy.Policy{}))(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + handler := RequireAdmin(policy.NewMemoryStore(&policy.Policy{}), testutil.NopLogger())(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) })) ctx := auth.WithRole(context.Background(), "admin") @@ -28,7 +28,7 @@ func TestRequireAdmin_AdminAllowed(t *testing.T) { func TestRequireAdmin_NonAdminForbidden(t *testing.T) { t.Parallel() - handler := RequireAdmin(policy.NewMemoryStore(&policy.Policy{}))(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + handler := RequireAdmin(policy.NewMemoryStore(&policy.Policy{}), testutil.NopLogger())(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { t.Fatal("handler should not be called") })) ctx := auth.WithRole(context.Background(), "viewer") @@ -44,7 +44,7 @@ func TestRequireAdmin_NonAdminForbidden(t *testing.T) { // admin route — fail closed with 403. func TestRequireAdmin_NoRoleForbidden(t *testing.T) { t.Parallel() - handler := RequireAdmin(nil)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + handler := RequireAdmin(nil, testutil.NopLogger())(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { t.Fatal("handler should not be called - a roleless request must not reach an admin route") })) req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil) @@ -59,7 +59,7 @@ func TestRequireAdmin_NoRoleForbidden(t *testing.T) { func TestRequireAdmin_CustomAdminRole(t *testing.T) { t.Parallel() store := policy.NewMemoryStore(&policy.Policy{AdminRole: "superuser"}) - handler := RequireAdmin(store)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + handler := RequireAdmin(store, testutil.NopLogger())(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) })) for role, want := range map[string]int{"superuser": http.StatusOK, "admin": http.StatusForbidden} { @@ -76,7 +76,7 @@ func TestRequireAdmin_CustomAdminRole(t *testing.T) { // (401) rather than a bare 403. func TestRequireAdmin_InvalidTokenFailsLoud(t *testing.T) { t.Parallel() - handler := RequireAdmin(nil)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + handler := RequireAdmin(nil, testutil.NopLogger())(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { t.Fatal("handler should not be called") })) ctx := auth.WithAuthError(context.Background(), errors.New("token expired")) @@ -252,13 +252,14 @@ func TestNewRouter_RoutesRegistered(t *testing.T) { hub := NewHub() deps := Dependencies{ - Ingest: NewIngestHandler(reg, pub), + Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, SSE: NewSSEHandler(hub, nil), WS: NewWSHandler(hub, nil, nil), Health: &HealthHandler{}, Schema: NewSchemaHandler(reg), AuthMW: func(next http.Handler) http.Handler { return next }, + Logger: testutil.NopLogger(), } router := NewRouter(deps) @@ -308,7 +309,7 @@ func TestNewRouter_RawSQLAdminGate(t *testing.T) { hub := NewHub() router := NewRouter(Dependencies{ - Ingest: NewIngestHandler(reg, pub), + Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, SSE: NewSSEHandler(hub, nil), WS: NewWSHandler(hub, nil, nil), @@ -316,6 +317,7 @@ func TestNewRouter_RawSQLAdminGate(t *testing.T) { Schema: NewSchemaHandler(reg), AuthMW: func(next http.Handler) http.Handler { return next }, PolicyStore: policy.NewMemoryStore(&policy.Policy{}), + Logger: testutil.NopLogger(), }) post := func(role string) *httptest.ResponseRecorder { @@ -369,7 +371,7 @@ func TestNewRouter_OptionalDepsNil(t *testing.T) { hub := NewHub() deps := Dependencies{ - Ingest: NewIngestHandler(reg, pub), + Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, SSE: NewSSEHandler(hub, nil), WS: NewWSHandler(hub, nil, nil), @@ -377,6 +379,7 @@ func TestNewRouter_OptionalDepsNil(t *testing.T) { Schema: NewSchemaHandler(reg), AuthMW: func(next http.Handler) http.Handler { return next }, PolicyStore: policy.NewMemoryStore(&policy.Policy{}), + Logger: testutil.NopLogger(), } // Should not panic. @@ -419,13 +422,14 @@ func TestNewRouter_NotFoundEmitsJSON(t *testing.T) { pub := &testutil.MockPublisher{} hub := NewHub() deps := Dependencies{ - Ingest: NewIngestHandler(reg, pub), + Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, SSE: NewSSEHandler(hub, nil), WS: NewWSHandler(hub, nil, nil), Health: &HealthHandler{}, Schema: NewSchemaHandler(reg), AuthMW: func(next http.Handler) http.Handler { return next }, + Logger: testutil.NopLogger(), } router := NewRouter(deps) @@ -444,13 +448,14 @@ func TestNewRouter_MethodNotAllowedEmitsJSON(t *testing.T) { pub := &testutil.MockPublisher{} hub := NewHub() deps := Dependencies{ - Ingest: NewIngestHandler(reg, pub), + Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, SSE: NewSSEHandler(hub, nil), WS: NewWSHandler(hub, nil, nil), Health: &HealthHandler{}, Schema: NewSchemaHandler(reg), AuthMW: func(next http.Handler) http.Handler { return next }, + Logger: testutil.NopLogger(), } router := NewRouter(deps) @@ -542,7 +547,7 @@ func TestNewRouter_SchemaAdminOnly(t *testing.T) { hub := NewHub() router := NewRouter(Dependencies{ - Ingest: NewIngestHandler(reg, pub), + Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, SSE: NewSSEHandler(hub, nil), WS: NewWSHandler(hub, nil, nil), @@ -550,6 +555,7 @@ func TestNewRouter_SchemaAdminOnly(t *testing.T) { Schema: NewSchemaHandler(reg), AuthMW: func(next http.Handler) http.Handler { return next }, PolicyStore: policy.NewMemoryStore(&policy.Policy{}), + Logger: testutil.NopLogger(), }) get := func(path, role string) *httptest.ResponseRecorder { diff --git a/internal/api/structured_query.go b/internal/api/structured_query.go index 8f3653fb..1f4f19d0 100644 --- a/internal/api/structured_query.go +++ b/internal/api/structured_query.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "time" @@ -25,6 +26,7 @@ type StructuredQueryHandler struct { BucketSecs int sf singleflight.Group maxQueryTimeout time.Duration + logger *slog.Logger } func NewStructuredQueryHandler( @@ -34,6 +36,7 @@ func NewStructuredQueryHandler( policyStore *policy.Store, bucketSecs int, queryTimeout time.Duration, + logger *slog.Logger, ) *StructuredQueryHandler { return &StructuredQueryHandler{ CHConn: conn, @@ -42,6 +45,7 @@ func NewStructuredQueryHandler( PolicyStore: policyStore, BucketSecs: bucketSecs, maxQueryTimeout: queryTimeout, + logger: logger, } } @@ -70,7 +74,11 @@ func (h *StructuredQueryHandler) Handle(w http.ResponseWriter, r *http.Request) claims, _ := auth.ClaimsFromContext(r.Context()) perms := policy.Evaluate(p, role, table, "select", claims) if !perms.Allowed { - writeAuthzDenied(w, r, role) + writeAuthzDenied(w, r, h.logger, role, nil, + slog.String("gate", "policy"), + slog.String("table", table), + slog.String("action", "select"), + ) return } diff --git a/internal/api/structured_query_test.go b/internal/api/structured_query_test.go index 846db985..87560545 100644 --- a/internal/api/structured_query_test.go +++ b/internal/api/structured_query_test.go @@ -39,7 +39,7 @@ func newStructuredQueryHandler() *StructuredQueryHandler { }, }, }) - return NewStructuredQueryHandler(nil, nil, reg, nil, 60, 5*time.Second) + return NewStructuredQueryHandler(nil, nil, reg, nil, 60, 5*time.Second, testutil.NopLogger()) } func TestStructuredQuery_MissingTable(t *testing.T) { diff --git a/tests/e2e/sdk/dlq.test.ts b/tests/e2e/sdk/dlq.test.ts index 877f8c21..1b6ff105 100644 --- a/tests/e2e/sdk/dlq.test.ts +++ b/tests/e2e/sdk/dlq.test.ts @@ -14,6 +14,9 @@ describe("Dead Letter Queue (DLQ) & Failures", () => { it("routes only the failed row to DLQ while valid rows are inserted", async () => { const runId = testId(); + // TODO: remove when #192 is fixed + await new Promise(resolve => setTimeout(resolve, 5000)); + // Get baseline DLQ stats before we pollute them const initialDlq = await admin.dlq.list(); const initialClicksDlq = diff --git a/tests/integration/setup_test.go b/tests/integration/setup_test.go index 0b072e06..bd73c73b 100644 --- a/tests/integration/setup_test.go +++ b/tests/integration/setup_test.go @@ -313,7 +313,7 @@ func buildServer(ch *chInstance, embeddedMQ *mq.EmbeddedNATS, registry *discover hub := api.NewHub() deps := api.Dependencies{ - Ingest: api.NewIngestHandler(registry, embeddedMQ), + Ingest: api.NewIngestHandler(registry, embeddedMQ, logger), // /v1/admin/query proxies straight to ClickHouse's HTTP interface, // so the handler needs the HTTP URL + creds rather than the // native-protocol driver.Conn other handlers use. @@ -329,7 +329,8 @@ func buildServer(ch *chInstance, embeddedMQ *mq.EmbeddedNATS, registry *discover next.ServeHTTP(w, r.WithContext(auth.WithRole(r.Context(), "admin"))) }) }, - JS: js, + JS: js, + Logger: logger, } server := httptest.NewServer(api.NewRouter(deps))