Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e34a4a3
fix(pipes): added test cases that expose AllowedRoles check being ski…
taitelee May 20, 2026
eb890cf
fix(pipes): fail closed on restricted pipes when role is missing
taitelee May 20, 2026
732b11d
main merge
taitelee May 20, 2026
c0e84f5
fix(policy): fail closed on empty roles and reject empty role key
taitelee May 21, 2026
7a62763
fixed test cases to match table-driven convention
taitelee May 21, 2026
cfce3ac
test(pipes,policy): cover pipe authorization and role-policy resoluti…
taitelee May 22, 2026
6a4afbf
fix(rbac): exact allowlist match, drop * wildcard, admin/service bypass
taitelee May 22, 2026
8faf824
make go tidy
taitelee May 22, 2026
32c2e81
docs(rbac): sync AGENTS/CHANGELOG/pipes comment with no-wildcard beha…
taitelee May 22, 2026
873dc39
docs(policy): drop stale *-grants claim in empty-role test comment
taitelee May 22, 2026
7a73df8
docs(agents): no-allowlist pipe denies all but admin/service; /v1/adm…
taitelee May 22, 2026
7d4c406
feat(auth): extract auth into internal/auth package
taitelee May 24, 2026
325021b
Merge origin/main: preserve RBAC auth/roles, adopt cache/table-name/o11y
taitelee May 24, 2026
fedf0e7
Merge remote-tracking branch 'origin/main' into rbac-security-enforce…
taitelee May 24, 2026
0054fe0
passing in policy to constructors
taitelee May 25, 2026
9464692
maintain original constructor signatures
taitelee May 25, 2026
ffe9b77
fixe error message for forbiddenForRole to be more clear
taitelee May 25, 2026
1ca1ff5
doc changes for clarification
taitelee May 25, 2026
6cd5ebb
docs(rbac): repoint orphaned middleware.go comment to internal/auth
taitelee May 25, 2026
3a0bdd6
docs(rbac): fix stale auth-model comments and ingest envelope scope
taitelee May 25, 2026
1739c0e
fix: addressed issues with admin role being able to be equivalent to …
taitelee May 26, 2026
8e66963
doc changes to include JWT vulnerability solutions
taitelee May 26, 2026
7836a86
Had to fix integration/setup_test.go to include new policystore changes
taitelee May 26, 2026
fe4309c
docs(auth): document header-over-query precedence, clarify parse fail…
taitelee May 26, 2026
c430255
Merge remote-tracking branch 'origin/main' into rbac-security-enforce…
taitelee May 26, 2026
87d3833
also removed that unnecessary case in resolveRole
taitelee May 26, 2026
4882b0d
changelog additions
taitelee May 26, 2026
e05bee3
changelog changes
taitelee May 27, 2026
0cadcc6
Added test cases for proper logging on denial and structured WARNs fo…
taitelee May 27, 2026
189240e
Merge remote-tracking branch 'origin/main' into rbac-security-enforce…
taitelee May 27, 2026
805bf1a
Merge remote-tracking branch 'origin/main' into rbac-security-enforce…
taitelee May 28, 2026
43f200a
feat(api): structured WARN on authorization denials via injected loggers
taitelee May 28, 2026
c3da09b
Removed LogLevel and untracked log level endpoint
taitelee May 29, 2026
aeb50b6
removed last reference of old log level var
taitelee Jun 1, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions cmd/wavehouse/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`).

Expand Down
4 changes: 2 additions & 2 deletions docs/src/content/docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading