From 7756e11ac46e9283d770617b9a69c37edb0e02fe Mon Sep 17 00:00:00 2001 From: taitelee Date: Tue, 7 Jul 2026 21:07:10 -0400 Subject: [PATCH 1/9] fix(stream): apply policy row-filter per subscriber on SSE --- CHANGELOG.md | 1 + cmd/wavehouse/main.go | 2 +- docs/src/content/docs/access-control.mdx | 4 +- docs/src/content/docs/architecture.md | 2 +- internal/api/errors_test.go | 2 +- internal/api/router_test.go | 14 +- internal/api/stream.go | 12 +- internal/api/stream_test.go | 8 +- internal/discovery/validation.go | 21 ++- internal/discovery/validation_test.go | 29 ++++ internal/policy/policy.go | 111 +++++++++++----- internal/policy/rowfilter.go | 121 +++++++++++++++++ internal/policy/rowfilter_test.go | 107 +++++++++++++++ internal/stream/hub.go | 160 ++++++++++++++++------- internal/stream/hub_test.go | 157 ++++++++++++++++++++-- internal/stream/subscriber.go | 13 ++ tests/integration/setup_test.go | 2 +- 17 files changed, 654 insertions(+), 112 deletions(-) create mode 100644 internal/policy/rowfilter.go create mode 100644 internal/policy/rowfilter_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b1ab641..faf59ded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security +- **Live SSE streams now apply a role's row-`filter` per subscriber, closing a query/stream row-level-security drift** (`internal/stream/hub.go`, `internal/stream/subscriber.go`, `internal/policy/policy.go`, `internal/policy/rowfilter.go` (new), `internal/discovery/validation.go`, `internal/api/stream.go`, `cmd/wavehouse/main.go`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/architecture.md`, plus tests in `internal/policy/rowfilter_test.go` (new), `internal/discovery/validation_test.go`, `internal/stream/hub_test.go`): closes #319. The SSE delivery path stripped denied columns but never applied a role's row-level `filter` predicate, so a subscriber received rows the structured-query path would have filtered out for that same role — a data-exposure on the streaming surface for any table that combines a row-policy with a shared or role-scoped stream (harmless on the public Stats table today, which carries no restrictive row-policy, but real for any private/PII table fronted by a stream). The row-filter is now resolved once into predicates that feed **both** read surfaces — the query path renders them to SQL, the stream evaluates them in memory (`ResolvedPermissions.RowVisible`) — so the two can't drift (the row-level analogue of the shared `IsColumnAllowed` decision from #223). Because a row-filter resolves against each subscriber's JWT claims, the #294/#353 once-per-role projection is now claims-aware: a role **without** a filter keeps the pure once-per-role fast path unchanged (zero regression on the public stream), while a role **with** a filter keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (evaluated against the full event, so a filter may key on a column the role can't select). Equality and set predicates (`_eq`/`_neq`/`_in` — the tenant/user scoping row-security actually uses) are enforced exactly; ordering predicates (`_gt`/`_lt`) are schema-informed — numeric columns compare numerically (matching ClickHouse), other types lexicographically — which is best-effort for exotic types stored as numbers (a documented boundary in access-control.mdx), with every ambiguous or uncomparable value failing closed (the row is withheld). Replay (gap-fill) applies the same per-connection row check. The resource limits (`max_rows`, `max_execution_time`, …) remain a query-path property and are still **not** applied to the stream (a separate, documented boundary). Supersedes the "stream path applies no row-level filter" invariant noted in the #294/#353 Changed entry below. - **Policy `_in` is now enforced on both the row-`filter` and insert-`check` paths, closing a fail-open row-security gap** (`internal/policy/policy.go`, `internal/api/ingest.go`, `docs/src/content/docs/access-control.mdx`, plus tests in `internal/policy/policy_test.go`, `internal/api/ingest_test.go`): closes #224. The `Filter` schema accepted `_in` but the engine never read it: on the row-`filter`/SELECT path `resolveFilters` had no `_in` branch, so a row-security filter like `tenant_id: { _in: … }` produced **no `WHERE` predicate** and the role saw every row instead of its tenant subset (a fail-open, same family as #223); on the `check`/INSERT path only `_eq` was honored, silently dropping any other operator. `_in` now takes a single claim that resolves to a JSON **array** (the multi-tenant case — a token's `tenant_ids` list) and emits `col IN (?, …)` with one bound param per element; a scalar claim is a one-element set, and an empty/absent claim matches **no rows** (fail-closed) rather than widening to all of them. On the insert path an `_in` check requires the column be present and one of the set — there is no single value to auto-inject as `_eq` does, so an omitted column is rejected (`403 check failed`). The comparison operators are enforced on `filter` (`_eq`/`_neq`/`_gt`/`_lt`/`_in` all produce predicates now, so nothing is rejected there) and, on `check`, `_neq`/`_gt`/`_lt` become a loud config-load rejection (no insert-time semantics; `check` honors `_eq` + `_in`). The `_in` value stays a single templated string in the wire schema (Go `Filter.In`, SDK `PolicyFilter._in`), matching the established "set = array" shape of the caller-query `in` operator. - **`denied_aggregations` is now enforced case-insensitively against the caller-supplied function name, closing a policy bypass** (`internal/policy/policy.go`, plus tests in `internal/policy/policy_test.go`): closes #318. `IsAggregationAllowed` lower-cased the aggregation name only *after* the deny-list loop and the empty-allow-list early return, so the deny check compared a lower-cased deny entry against the raw caller input — a denied aggregation slipped past simply by changing case (`SUM` bypassed a `sum` deny entry, and with an empty allow list the call was then permitted). `isValidAggFn` already accepts any casing and the SQL builder emits the function name verbatim, so the denied aggregation actually executed. The case fold now happens once, before the deny check, so deny wins regardless of caller casing — matching the case-insensitive contract the access-control docs already specified. - **A role's structured-query resource caps are now enforced server-side by ClickHouse, so a public read can't outrun its budget during a server-side scan/merge/aggregation phase** (`internal/api/ch_settings.go` (new), `internal/api/structured_query.go`, `internal/policy/policy.go`, `internal/policy/scalars.go` (new), `internal/config/config.go`, `internal/query/builder.go`, `cmd/wavehouse/main.go`, `config.yaml`, `clients/ts/src/types.ts`, `docs/src/content/docs/{access-control.mdx,configuration.mdx}`, plus tests in `internal/api/ch_settings_test.go`, `internal/policy/{policy,scalars}_test.go`, `internal/config/config_test.go`, `internal/query/builder_test.go`, `tests/integration/query_limits_test.go` (all new/expanded), `tests/e2e/sdk/query.test.ts`): closes #316. The native ClickHouse connection passed **no per-query `Settings`**, so a read's policy caps bound it only client-side — a Go `context` deadline (which cancels only while the client is reading result blocks) plus a SQL `LIMIT`. Memory and rows scanned were **never bounded server-side**, so a heavy aggregation could allocate gigabytes of state or scan an entire table well within the time budget; and because `clickhouse-go` derives a `max_execution_time` setting from the context deadline only for deadlines `> 1s`, a sub-second time cap reached ClickHouse with no server-side time bound at all. The structured-query path now attaches per-query `Settings` derived from the role's resolved permissions: `max_execution_time` (fractional seconds, emitted explicitly so the sub-second case is enforced), `max_result_rows` + `result_overflow_mode=throw` (defense-in-depth behind the SQL `LIMIT`), `max_rows_to_read` + `read_overflow_mode=throw`, and `max_memory_usage` — so a query that exceeds its budget is rejected by the server (ClickHouse codes 158 / 241) rather than running to completion. **Boundary:** WaveHouse owns the *dynamic, per-role* caps (sent as per-query settings); the *global, static* backstop — which applies to every query including named pipes and raw admin SQL — is configured in **ClickHouse's own settings profiles and quotas** (documented in `configuration.mdx`), composes with the per-role caps, and holds even against a WaveHouse bug. **Schema (pre-launch):** the per-role policy fields are human-readable in / numeric out — `max_execution_time_ms` (int) → **`max_execution_time`** (set as a duration string `"5s"` or a bare ms number; read back as ms), the new **`max_memory_usage`** (set as a size string `"4GiB"` — IEC/SI respected via `dustin/go-humanize`, so `4GB` ≠ `4GiB` — or a bare byte number; read back as bytes), and the new **`max_rows_to_read`** (int), backed by two small `Millis`/`ByteSize` types in the `policy` package. The formerly hard-coded `query.DefaultMaxRows = 10000` result-LIMIT becomes the documented, tunable `query.default_max_rows` config knob (`Build` takes it as a parameter). Raw admin SQL remains unbounded by WaveHouse (governed by ClickHouse). Verified RED before the fix: the handler-level integration test confirmed a capped read returned the full result set when the settings weren't sent. diff --git a/cmd/wavehouse/main.go b/cmd/wavehouse/main.go index c75eb853..7ae3e21a 100644 --- a/cmd/wavehouse/main.go +++ b/cmd/wavehouse/main.go @@ -292,7 +292,7 @@ func run() int { // handler (write counts), and the Hub that projects/serializes each event once // per (topic, role) and pushes it to that role's subscribers. sseMetrics := stream.NewMetrics() - streamHub := stream.NewHub(policyStore, sseMetrics) + streamHub := stream.NewHub(policyStore, registry, sseMetrics) // Start policy watch for cluster-wide updates. go policyStore.Watch(ctx) diff --git a/docs/src/content/docs/access-control.mdx b/docs/src/content/docs/access-control.mdx index dda1e0b5..13664a15 100644 --- a/docs/src/content/docs/access-control.mdx +++ b/docs/src/content/docs/access-control.mdx @@ -374,8 +374,8 @@ The same policy drives every data path, but not every field is meaningful on eve | Raw SQL | `POST /v1/admin/query` | `admin_role` only — no per-statement policy; the role gate is the entire authorization story | | Named pipe | `GET/POST /v1/pipes/{name}` | per-pipe `allowed_roles` (not the policy engine; see [Named Pipes](/pipes)). Resource limits come from ClickHouse's [server-wide settings](/configuration#server-side-resource-limits), not per-role policy caps | -:::caution[Live streams enforce access, not row filters] -SSE subscribers are checked for table-level `select` permission and have denied columns stripped from each event, but the row-level `filter` predicates and the resource limits (`max_rows`, `max_execution_time`, `max_rows_to_read`, `max_memory_usage`) are a property of the SQL query path and are **not** applied to the live event stream. If a role must never observe another tenant's rows in real time, don't grant it stream access to a shared table — scope the data at the table level. +:::caution[Live streams enforce column and row policy, but not resource limits] +SSE subscribers are checked for table-level `select` permission, have denied columns stripped from each event, and — like the query path — receive only the rows their role's row-level `filter` predicates admit, evaluated per subscriber against their JWT claims. Equality and set predicates (`_eq`, `_neq`, `_in`) — the usual tenant/user scoping — are enforced exactly. Ordering predicates (`_gt`, `_lt`) are best-effort: the stream compares numeric columns numerically (matching ClickHouse) but lacks ClickHouse's full per-type coercion, so an ordering filter on an exotic type (a `Decimal`/`Int128` beyond `float64` precision, or a `DateTime` ingested as a Unix number) can admit or withhold a row the query path wouldn't. The resource limits (`max_rows`, `max_execution_time`, `max_rows_to_read`, `max_memory_usage`) remain a property of the SQL query path and are **not** applied to the live event stream; if those caps are part of a role's isolation story, don't rely on them over the stream. ::: ## Managing the policy diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index 4c6a8958..f35d5e80 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -85,7 +85,7 @@ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with Request The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https://github.com/Wave-RF/WaveHouse/issues/294)) lives next to the keepalive primitives it shares. One abstraction per file. -- **hub.go** — `Hub`, the event fan-out. Subscribers register under `(topic, role)`; `Broadcast` decodes each event once, applies each subscribed role's column policy once, builds one SSE frame per role, and fans it to every member of that role's `Bucket` — collapsing the prior per-subscriber `unmarshal → evaluate → filter → marshal` into one pass per distinct `(role, table)` output shape (the [#294](https://github.com/Wave-RF/WaveHouse/issues/294) lever; the measured ceiling was ~2 270 deliveries/s from re-projecting per subscriber). The `(topic, role)` key is sufficient because column visibility derives only from the role+table policy entry, never from JWT claims (claims feed only the row-level `WHERE`/`CHECK`, which the stream path does not apply). `ReplayFrame` shares the same projection for the handler's per-connection gap-fill. +- **hub.go** — `Hub`, the event fan-out. Subscribers register under `(topic, role)`; `Broadcast` decodes each event once, applies each subscribed role's column policy once, builds one SSE frame per role, and fans it to every member of that role's `Bucket` — collapsing the prior per-subscriber `unmarshal → evaluate → filter → marshal` into one pass per distinct `(role, table)` output shape (the [#294](https://github.com/Wave-RF/WaveHouse/issues/294) lever; the measured ceiling was ~2 270 deliveries/s from re-projecting per subscriber). The column projection is claims-independent, so it is shared across a role's whole bucket; the role's row-level `filter` predicate is not — it is resolved against each subscriber's JWT claims, so for a role that carries a filter `Broadcast` keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (`ResolvedPermissions.RowVisible`, evaluated against the full event via the numeric-aware comparison seeded from the schema registry). This is the [#319](https://github.com/Wave-RF/WaveHouse/issues/319) fix that closes the query/stream row-level-security drift; roles without a filter keep the pure once-per-role fast path. `ReplayFrame` shares the same projection and per-connection row check for the handler's gap-fill. - **subscriber.go** — `Subscriber`, the per-connection handle. It owns a single ready-to-write outbound queue of `Frame`s (each tagged with its `kind`, so the handler labels the write where it happens): producers — the keepalive wheel and the event `Hub` — fan frames in with `Send` (non-blocking; a full queue drops and the `Hub` counts it), and the handler drains `Frames()` to the client verbatim. The queue is sized for buffering live events (cap 64, up from the keepalive-only cap 1; #152 will make it a knob), and an `Evicted()` channel is the seam the slow-consumer follow-up closes to disconnect a wedged consumer. - **bucket.go** — `Bucket`, the reusable fan-out primitive: a concurrency-safe set of subscribers. `Push` delivers a shared `Frame` to each fire-and-forget (the keepalive wheel's ring); `Snapshot` exposes the members so the event `Hub` can fan out while inspecting each `Send` result (to count drops). The `Hub` holds one `Bucket` per `(topic, role)` so a projected frame is built once and sent to every member instead of re-projected per subscriber. - **heartbeat.go** — The keepalive wheel (`Heartbeater`). A single process-wide ticker fans a minimal `:` comment across the ring of `Bucket`s, waking ~1/N of live streams per tick so the writes don't synchronize. The effective per-connection keepalive period is `stream.keepalive_interval` (the wheel ticks every `keepalive_interval ÷ keepalive_buckets`, so one rotation spans the interval); the owning handler goroutine does the actual write, so the shared ticker never touches a `ResponseWriter` directly. diff --git a/internal/api/errors_test.go b/internal/api/errors_test.go index 358a3f34..3f2956e7 100644 --- a/internal/api/errors_test.go +++ b/internal/api/errors_test.go @@ -194,7 +194,7 @@ func TestAuthzDenied_LogsChiRoutePattern(t *testing.T) { router := NewRouter(Dependencies{ Ingest: NewIngestHandler(reg, &testutil.MockPublisher{}, logger), Query: &QueryHandler{}, - SSE: NewStreamHandler(stream.NewHub(nil, nil), nil), + SSE: NewStreamHandler(stream.NewHub(nil, nil, nil), nil), Health: &HealthHandler{}, Schema: NewSchemaHandler(reg), AuthMW: func(next http.Handler) http.Handler { return next }, diff --git a/internal/api/router_test.go b/internal/api/router_test.go index bf9dcaac..a0c1d5ac 100644 --- a/internal/api/router_test.go +++ b/internal/api/router_test.go @@ -283,7 +283,7 @@ func TestNewRouter_RoutesRegistered(t *testing.T) { {Name: "events", Columns: []discovery.Column{{Name: "id", Type: "String"}}}, }) pub := &testutil.MockPublisher{} - hub := stream.NewHub(nil, nil) + hub := stream.NewHub(nil, nil, nil) deps := Dependencies{ Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), @@ -339,7 +339,7 @@ func TestNewRouter_RoutesRegistered(t *testing.T) { func TestNewRouter_CORSOnStream(t *testing.T) { t.Parallel() - hub := stream.NewHub(nil, nil) + hub := stream.NewHub(nil, nil, nil) router := NewRouter(Dependencies{ SSE: NewStreamHandler(hub, nil), Health: &HealthHandler{}, @@ -411,7 +411,7 @@ func TestNewRouter_RawSQLAdminGate(t *testing.T) { reg := discovery.NewSchemaRegistryFromMap(nil) pub := &testutil.MockPublisher{} - hub := stream.NewHub(nil, nil) + hub := stream.NewHub(nil, nil, nil) router := NewRouter(Dependencies{ Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), @@ -472,7 +472,7 @@ func TestNewRouter_OptionalDepsNil(t *testing.T) { reg := discovery.NewSchemaRegistryFromMap(nil) pub := &testutil.MockPublisher{} - hub := stream.NewHub(nil, nil) + hub := stream.NewHub(nil, nil, nil) deps := Dependencies{ Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), @@ -523,7 +523,7 @@ func TestNewRouter_NotFoundEmitsJSON(t *testing.T) { reg := discovery.NewSchemaRegistryFromMap(nil) pub := &testutil.MockPublisher{} - hub := stream.NewHub(nil, nil) + hub := stream.NewHub(nil, nil, nil) deps := Dependencies{ Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, @@ -548,7 +548,7 @@ func TestNewRouter_MethodNotAllowedEmitsJSON(t *testing.T) { reg := discovery.NewSchemaRegistryFromMap(nil) pub := &testutil.MockPublisher{} - hub := stream.NewHub(nil, nil) + hub := stream.NewHub(nil, nil, nil) deps := Dependencies{ Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), Query: &QueryHandler{}, @@ -645,7 +645,7 @@ func TestNewRouter_SchemaAdminOnly(t *testing.T) { t.Parallel() reg := discovery.NewSchemaRegistryFromMap(nil) pub := &testutil.MockPublisher{} - hub := stream.NewHub(nil, nil) + hub := stream.NewHub(nil, nil, nil) router := NewRouter(Dependencies{ Ingest: NewIngestHandler(reg, pub, testutil.NopLogger()), diff --git a/internal/api/stream.go b/internal/api/stream.go index 44485224..a0727573 100644 --- a/internal/api/stream.go +++ b/internal/api/stream.go @@ -42,10 +42,13 @@ func (h *StreamHandler) Handle(w http.ResponseWriter, r *http.Request) { } // Resolve stream permissions for this request. The raw role from context is the - // bucket key: the Hub projects/serializes once per (topic, role), and column - // visibility derives only from the role+table policy entry — never from claims - // (see stream.Hub). Evaluate maps an empty role to the policy default_role. + // bucket key: the Hub serializes the column projection once per (topic, role), + // since column visibility derives only from the role+table policy entry. Claims + // are separate — they drive the row-level-security filter, which the Hub evaluates + // per subscriber (see stream.Hub), so they ride on the Subscriber rather than the + // bucket key. Evaluate maps an empty role to the policy default_role. role := auth.RoleFromContext(r.Context()) + claims, _ := auth.ClaimsFromContext(r.Context()) // TODO: impl scope scope := "" @@ -79,6 +82,7 @@ func (h *StreamHandler) Handle(w http.ResponseWriter, r *http.Request) { // buffer in the subscriber queue instead of being missed (an overlap with // replay yields duplicates, deduped client-side by id: — at-least-once). sub := stream.NewSubscriber() + sub.SetClaims(claims) h.Hub.Add(topic, role, sub) defer h.Hub.Remove(topic, role, sub) @@ -96,7 +100,7 @@ func (h *StreamHandler) Handle(w http.ResponseWriter, r *http.Request) { // the replayed frame. A write error means the client is gone, so stop the // gap-fill and let the deferred cleanup unwind. sendReplay := func(data []byte) bool { - f, ok := h.Hub.ReplayFrame(role, data) + f, ok := h.Hub.ReplayFrame(role, claims, data) if !ok { return true // filtered for this role — skip } diff --git a/internal/api/stream_test.go b/internal/api/stream_test.go index 5d67b763..88db1d99 100644 --- a/internal/api/stream_test.go +++ b/internal/api/stream_test.go @@ -22,7 +22,7 @@ import ( func TestSSE_RejectsMissingOrInvalidTable(t *testing.T) { t.Parallel() - h := &StreamHandler{Hub: stream.NewHub(nil, nil)} + h := &StreamHandler{Hub: stream.NewHub(nil, nil, nil)} cases := []struct { name string @@ -50,7 +50,7 @@ func TestSSE_RejectsMissingOrInvalidTable(t *testing.T) { func TestSSE_AcceptsSafeTableName(t *testing.T) { t.Parallel() - h := &StreamHandler{Hub: stream.NewHub(nil, nil)} + h := &StreamHandler{Hub: stream.NewHub(nil, nil, nil)} // Use a request context that's already cancelled so the handler exits // the live-stream select loop immediately instead of blocking the test. @@ -75,7 +75,7 @@ func TestSSE_EmitsHeartbeatsWhenIdle(t *testing.T) { hb := stream.NewHeartbeater(20*time.Millisecond, 1) go hb.Run(t.Context()) - h := &StreamHandler{Hub: stream.NewHub(nil, nil), Heartbeater: hb} + h := &StreamHandler{Hub: stream.NewHub(nil, nil, nil), Heartbeater: hb} ctx, cancel := context.WithCancel(context.Background()) req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/v1/stream?table=clicks", nil) @@ -114,7 +114,7 @@ func TestSSE_WheelTickRacesHandlerTeardown(t *testing.T) { defer cancel() go hb.Run(ctx) - h := &StreamHandler{Hub: stream.NewHub(nil, nil), Heartbeater: hb} + h := &StreamHandler{Hub: stream.NewHub(nil, nil, nil), Heartbeater: hb} const conns = 40 var wg sync.WaitGroup diff --git a/internal/discovery/validation.go b/internal/discovery/validation.go index 90af29be..b2dbdf51 100644 --- a/internal/discovery/validation.go +++ b/internal/discovery/validation.go @@ -48,9 +48,9 @@ func Validate(schema *TableSchema, data map[string]any) error { return nil } -// isTypeCompatible checks whether a Go/JSON value can be stored in the given ClickHouse type. -func isTypeCompatible(chType string, val any) bool { - // Robustly unwrap nested modifiers (e.g., LowCardinality(Nullable(String))) +// unwrapType strips Nullable(...) and LowCardinality(...) modifiers (nested in any +// order, e.g. LowCardinality(Nullable(String))) down to the base ClickHouse type. +func unwrapType(chType string) string { for { if strings.HasPrefix(chType, "Nullable(") && strings.HasSuffix(chType, ")") { chType = chType[9 : len(chType)-1] @@ -60,8 +60,13 @@ func isTypeCompatible(chType string, val any) bool { chType = chType[15 : len(chType)-1] continue } - break + return chType } +} + +// isTypeCompatible checks whether a Go/JSON value can be stored in the given ClickHouse type. +func isTypeCompatible(chType string, val any) bool { + chType = unwrapType(chType) switch { // String-compatible types accept Strings, Numbers (coerced), and Bools @@ -144,6 +149,14 @@ func isTypeCompatible(chType string, val any) bool { } } +// IsNumericType reports whether chType is a ClickHouse numeric type (integer, float, +// or decimal), unwrapping Nullable/LowCardinality modifiers first. The stream +// row-filter evaluator uses it to choose numeric vs lexicographic comparison for a +// column, so ordering predicates (>, <) on numbers match ClickHouse (9 < 100). +func IsNumericType(chType string) bool { + return isNumericType(unwrapType(chType)) +} + // isNumericType returns true for ClickHouse integer, float, and decimal types. func isNumericType(chType string) bool { switch { diff --git a/internal/discovery/validation_test.go b/internal/discovery/validation_test.go index 1dbbb08a..4a4015d1 100644 --- a/internal/discovery/validation_test.go +++ b/internal/discovery/validation_test.go @@ -224,3 +224,32 @@ func TestIsNumericType(t *testing.T) { }) } } + +// TestIsNumericType_Exported checks the exported wrapper the stream row-filter uses: +// it must unwrap Nullable/LowCardinality (in any nesting) before classifying, so a +// Nullable(UInt64) column still compares numerically. +func TestIsNumericType_Exported(t *testing.T) { + t.Parallel() + + tests := []struct { + chType string + want bool + }{ + {"UInt64", true}, + {"Nullable(UInt64)", true}, + {"LowCardinality(Int32)", true}, + {"LowCardinality(Nullable(Float64))", true}, + {"Decimal(10,2)", true}, + {"String", false}, + {"Nullable(String)", false}, + {"LowCardinality(String)", false}, + {"DateTime", false}, + } + + for _, tt := range tests { + t.Run(tt.chType, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, IsNumericType(tt.chType)) + }) + } +} diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 198bc5a3..c0f5a4f3 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -62,11 +62,16 @@ type Filter struct { // ResolvedPermissions is the result of evaluating a policy against JWT claims. type ResolvedPermissions struct { - Allowed bool - AllowColumns []string - DenyColumns []string - WhereClause string - WhereParams []any + Allowed bool + AllowColumns []string + DenyColumns []string + WhereClause string + WhereParams []any + // rowFilter is the same row-level-security predicate as WhereClause/WhereParams, + // kept in resolved form so the stream path can evaluate it in memory (RowVisible) + // while the query path renders it to SQL. Both derive from one resolvePredicates + // call in Evaluate, so the two read surfaces can't drift. See rowfilter.go. + rowFilter []resolvedPredicate CheckClauses map[string]any // column → required value (for inserts) AllowedAggregations []string DeniedAggregations []string @@ -189,7 +194,13 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * return &ResolvedPermissions{Allowed: false} } } - clauses, params := resolveFilters(perms.Filter, claims) + // Resolve the row-filter once into predicates, then render both read surfaces + // from that single source so they can't drift: the query path binds them into + // a SQL WHERE here; the stream path evaluates the same predicates in memory + // (ResolvedPermissions.RowVisible). + preds := resolvePredicates(perms.Filter, claims) + resolved.rowFilter = preds + clauses, params := predicatesToSQL(preds) if len(clauses) > 0 { resolved.WhereClause = strings.Join(clauses, " AND ") resolved.WhereParams = params @@ -214,52 +225,92 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * return resolved } -// resolveFilters converts filter definitions with claim templates into SQL WHERE clauses. -func resolveFilters(filters map[string]Filter, claims map[string]any) ([]string, []any) { - var clauses []string - var params []any +// resolvedPredicate is one row-filter comparison with its claim templates already +// resolved to concrete string values — the shared, render-agnostic form the query +// path turns into SQL (predicatesToSQL) and the stream path evaluates in memory +// (RowVisible). Op is one of "=", "!=", ">", "<", "in". Values holds one element +// for the scalar operators and zero-or-more for "in" (empty ⇒ matches no rows). +type resolvedPredicate struct { + Column string + Op string + Values []string +} + +// resolvePredicates resolves each filter's claim templates once into predicates. +// Both read surfaces derive from this single result so they can't drift; the +// operator order within a column (=, !=, >, <, in) mirrors the former inline SQL. +func resolvePredicates(filters map[string]Filter, claims map[string]any) []resolvedPredicate { + var preds []resolvedPredicate for col, f := range filters { - // Quote the policy-authored column the same way the query builder quotes - // caller columns, so a row-filter on a weird-but-legal column name (dots, - // spaces, keywords) is emitted safely. - qcol := chsql.QuoteIdent(col) if f.Eq != nil { - val := resolveTemplate(*f.Eq, claims) - clauses = append(clauses, fmt.Sprintf("%s = ?", qcol)) - params = append(params, val) + preds = append(preds, resolvedPredicate{col, "=", []string{resolveTemplate(*f.Eq, claims)}}) } if f.Neq != nil { - val := resolveTemplate(*f.Neq, claims) - clauses = append(clauses, fmt.Sprintf("%s != ?", qcol)) - params = append(params, val) + preds = append(preds, resolvedPredicate{col, "!=", []string{resolveTemplate(*f.Neq, claims)}}) } if f.Gt != nil { - val := resolveTemplate(*f.Gt, claims) - clauses = append(clauses, fmt.Sprintf("%s > ?", qcol)) - params = append(params, val) + preds = append(preds, resolvedPredicate{col, ">", []string{resolveTemplate(*f.Gt, claims)}}) } if f.Lt != nil { - val := resolveTemplate(*f.Lt, claims) - clauses = append(clauses, fmt.Sprintf("%s < ?", qcol)) - params = append(params, val) + preds = append(preds, resolvedPredicate{col, "<", []string{resolveTemplate(*f.Lt, claims)}}) } if f.In != nil { - vals := resolveInValues(*f.In, claims) - if len(vals) == 0 { + preds = append(preds, resolvedPredicate{col, "in", toStrings(resolveInValues(*f.In, claims))}) + } + } + return preds +} + +// predicatesToSQL renders resolved predicates into WHERE clauses and bound params. +func predicatesToSQL(preds []resolvedPredicate) ([]string, []any) { + var clauses []string + var params []any + for _, p := range preds { + // Quote the policy-authored column the same way the query builder quotes + // caller columns, so a row-filter on a weird-but-legal column name (dots, + // spaces, keywords) is emitted safely. + qcol := chsql.QuoteIdent(p.Column) + switch p.Op { + case "in": + if len(p.Values) == 0 { // An empty/unresolvable set fails closed: a row filter scoped to no // values matches no rows, never widening to all of them (the #224 // fail-open). `IN ()` is not valid SQL, so emit a constant false. clauses = append(clauses, "1 = 0") } else { - placeholders := strings.TrimSuffix(strings.Repeat("?,", len(vals)), ",") + placeholders := strings.TrimSuffix(strings.Repeat("?,", len(p.Values)), ",") clauses = append(clauses, fmt.Sprintf("%s IN (%s)", qcol, placeholders)) - params = append(params, vals...) + for _, v := range p.Values { + params = append(params, v) + } } + default: + clauses = append(clauses, fmt.Sprintf("%s %s ?", qcol, p.Op)) + params = append(params, p.Values[0]) } } return clauses, params } +// resolveFilters converts filter definitions with claim templates into SQL WHERE +// clauses. Retained as the predicates→SQL composition the query-path tests target. +func resolveFilters(filters map[string]Filter, claims map[string]any) ([]string, []any) { + return predicatesToSQL(resolvePredicates(filters, claims)) +} + +// toStrings normalizes resolveInValues' []any (already stringified elements) to the +// []string a resolvedPredicate carries. +func toStrings(vals []any) []string { + if len(vals) == 0 { + return nil + } + out := make([]string, len(vals)) + for i, v := range vals { + out[i] = fmt.Sprint(v) + } + return out +} + // resolveTemplate resolves {{ jwt.claim.path }} templates against JWT claims. // If a claim path cannot be resolved, the template placeholder is replaced with // an empty string to prevent "" from leaking into SQL filters. diff --git a/internal/policy/rowfilter.go b/internal/policy/rowfilter.go new file mode 100644 index 00000000..efaf2f05 --- /dev/null +++ b/internal/policy/rowfilter.go @@ -0,0 +1,121 @@ +package policy + +import ( + "strconv" + "strings" +) + +// HasRowFilter reports whether this role/table entry carries a row-level-security +// predicate. The stream fan-out uses it to decide whether an event can be projected +// once for a whole role bucket (no filter) or must be checked per subscriber against +// that subscriber's claims (filter present). A nil receiver (no policy applies) has +// no filter. +func (p *ResolvedPermissions) HasRowFilter() bool { + return p != nil && len(p.rowFilter) > 0 +} + +// RowVisible reports whether row satisfies every resolved row-filter predicate — the +// in-memory twin of the query path's WHERE clause, evaluated against a decoded event +// so the stream applies the same row-level security the query path does. Predicates +// are ANDed; the query path joins them with AND too. +// +// numericCols maps column name → true when that column's ClickHouse type is numeric +// (the caller supplies it from the table schema; see discovery.IsNumericType). +// Numeric columns compare numerically, so 9 < 100 as ClickHouse would; every other +// column — and any column absent from the map, e.g. when no schema is available — +// compares as text. This is exact for the equality/set operators row-level security +// actually uses (=, !=, in) and best-effort for ordering (>, <): it cannot perfectly +// mirror ClickHouse's per-type coercion for exotic types (Decimal / Int128 beyond +// float64 precision, Date/DateTime stored as Unix numbers). Every ambiguous or +// uncomparable case fails closed — the row is hidden, never leaked — so the boundary +// costs availability, not confidentiality. +// +// A nil receiver (no policy applies) makes every row visible. +func (p *ResolvedPermissions) RowVisible(row map[string]any, numericCols map[string]bool) bool { + if p == nil { + return true + } + for _, pred := range p.rowFilter { + if !pred.matches(row, numericCols[pred.Column]) { + return false + } + } + return true +} + +// matches evaluates one predicate against the row, failing closed (false) whenever +// the value is absent or can't be compared as required. +func (pred resolvedPredicate) matches(row map[string]any, numeric bool) bool { + raw, ok := row[pred.Column] + if !ok { + return false // column not in the event ⇒ can't prove the row is allowed + } + switch pred.Op { + case "=": + c, ok := compareScalar(raw, pred.Values[0], numeric) + return ok && c == 0 + case "!=": + c, ok := compareScalar(raw, pred.Values[0], numeric) + return ok && c != 0 + case ">": + c, ok := compareScalar(raw, pred.Values[0], numeric) + return ok && c > 0 + case "<": + c, ok := compareScalar(raw, pred.Values[0], numeric) + return ok && c < 0 + case "in": + for _, v := range pred.Values { + if c, ok := compareScalar(raw, v, numeric); ok && c == 0 { + return true + } + } + return false + default: + return false + } +} + +// compareScalar compares an event value against a resolved filter value, returning +// -1/0/+1 and ok=false when the comparison can't be made (a non-scalar event value, +// or a numeric comparison whose operands don't parse as numbers). numeric selects +// numeric vs lexicographic ordering. +func compareScalar(rowVal any, filterVal string, numeric bool) (int, bool) { + s, ok := scalarString(rowVal) + if !ok { + return 0, false + } + if numeric { + a, err1 := strconv.ParseFloat(s, 64) + b, err2 := strconv.ParseFloat(filterVal, 64) + if err1 != nil || err2 != nil { + return 0, false + } + switch { + case a < b: + return -1, true + case a > b: + return 1, true + default: + return 0, true + } + } + return strings.Compare(s, filterVal), true +} + +// scalarString renders a JSON-decoded scalar as the canonical string compared +// against a (string-valued) filter. Non-scalars (arrays, objects, null) return +// ok=false so the predicate fails closed rather than guessing. JSON numbers arrive +// as float64 via encoding/json; -1 precision emits the shortest round-trip form +// without an exponent, so integer IDs read back as "123", not "1.23e+02". +func scalarString(v any) (string, bool) { + switch x := v.(type) { + case string: + return x, true + case float64: + return strconv.FormatFloat(x, 'f', -1, 64), true + case bool: + return strconv.FormatBool(x), true + default: + return "", false + } +} diff --git a/internal/policy/rowfilter_test.go b/internal/policy/rowfilter_test.go new file mode 100644 index 00000000..5b3b39a8 --- /dev/null +++ b/internal/policy/rowfilter_test.go @@ -0,0 +1,107 @@ +package policy + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// evalRowFilter builds a one-role/one-table policy carrying filter and returns the +// permissions resolved against claims, so tests exercise the full +// resolvePredicates → RowVisible path the stream fan-out uses. +func evalRowFilter(t *testing.T, filter map[string]Filter, claims map[string]any) *ResolvedPermissions { + t.Helper() + p := &Policy{Tables: map[string]TablePolicy{ + "t": {Select: map[string]RolePermissions{"r": {Filter: filter}}}, + }} + return Evaluate(p, "r", "t", "select", claims) +} + +// TestRowVisible_EqualityScoping covers the canonical row-level-security shape — +// tenant_id = {{ jwt.tenant }} — including the fail-closed on a missing column that +// stops an event without the filtered field from slipping through. +func TestRowVisible_EqualityScoping(t *testing.T) { + t.Parallel() + perms := evalRowFilter(t, map[string]Filter{"tenant_id": {Eq: new("{{ jwt.tenant }}")}}, map[string]any{"tenant": "acme"}) + is := assert.New(t) + is.True(perms.HasRowFilter()) + is.True(perms.RowVisible(map[string]any{"tenant_id": "acme"}, nil)) + is.False(perms.RowVisible(map[string]any{"tenant_id": "globex"}, nil)) + is.False(perms.RowVisible(map[string]any{"other": "x"}, nil), "missing filtered column ⇒ fail closed") +} + +// TestRowVisible_InSet covers _in against an array claim (multi-tenant scoping) and +// the fail-closed empty-set case (absent claim never widens to all rows). +func TestRowVisible_InSet(t *testing.T) { + t.Parallel() + perms := evalRowFilter(t, map[string]Filter{"tenant_id": {In: new("{{ jwt.tenants }}")}}, map[string]any{"tenants": []any{"a", "b"}}) + assert.True(t, perms.RowVisible(map[string]any{"tenant_id": "b"}, nil)) + assert.False(t, perms.RowVisible(map[string]any{"tenant_id": "c"}, nil)) + + empty := evalRowFilter(t, map[string]Filter{"tenant_id": {In: new("{{ jwt.tenants }}")}}, nil) + assert.True(t, empty.HasRowFilter()) + assert.False(t, empty.RowVisible(map[string]any{"tenant_id": "a"}, nil), "absent claim ⇒ empty set ⇒ no row matches") +} + +func TestRowVisible_Neq(t *testing.T) { + t.Parallel() + perms := evalRowFilter(t, map[string]Filter{"status": {Neq: new("deleted")}}, nil) + assert.True(t, perms.RowVisible(map[string]any{"status": "active"}, nil)) + assert.False(t, perms.RowVisible(map[string]any{"status": "deleted"}, nil)) +} + +// TestRowVisible_Ordering_NumericVsLexicographic is the schema-informed behavior: an +// event value of 9 is numerically LESS than 100 but lexicographically GREATER +// ("9" > "100"). With the column marked numeric the row is hidden (matching +// ClickHouse); the no-schema lexicographic fallback would leak it — the footgun the +// schema closes. +func TestRowVisible_Ordering_NumericVsLexicographic(t *testing.T) { + t.Parallel() + perms := evalRowFilter(t, map[string]Filter{"amount": {Gt: new("100")}}, nil) + row := map[string]any{"amount": float64(9)} + + assert.False(t, perms.RowVisible(row, map[string]bool{"amount": true}), "numeric: 9 is not > 100") + assert.True(t, perms.RowVisible(row, nil), `lexicographic fallback: "9" > "100"`) + assert.True(t, perms.RowVisible(map[string]any{"amount": float64(250)}, map[string]bool{"amount": true})) +} + +// TestRowVisible_NumericEquality_FloatFormatting: a JSON float64(100) equals the +// string filter value "100" under numeric comparison, so integer-valued numeric +// columns aren't tripped up by float formatting. +func TestRowVisible_NumericEquality_FloatFormatting(t *testing.T) { + t.Parallel() + perms := evalRowFilter(t, map[string]Filter{"amount": {Eq: new("100")}}, nil) + num := map[string]bool{"amount": true} + assert.True(t, perms.RowVisible(map[string]any{"amount": float64(100)}, num)) + assert.False(t, perms.RowVisible(map[string]any{"amount": float64(101)}, num)) +} + +func TestRowVisible_NilReceiver_AllVisible(t *testing.T) { + t.Parallel() + var perms *ResolvedPermissions + assert.True(t, perms.RowVisible(map[string]any{"x": "y"}, nil)) + assert.False(t, perms.HasRowFilter()) +} + +// TestRowVisible_NonScalar_FailsClosed: a nested/array/null event value can't be +// compared to a scalar filter value, so the predicate fails closed rather than guess. +func TestRowVisible_NonScalar_FailsClosed(t *testing.T) { + t.Parallel() + perms := evalRowFilter(t, map[string]Filter{"tenant_id": {Eq: new("acme")}}, nil) + assert.False(t, perms.RowVisible(map[string]any{"tenant_id": []any{"acme"}}, nil)) + assert.False(t, perms.RowVisible(map[string]any{"tenant_id": nil}, nil)) +} + +// TestRowVisible_MultiplePredicates_AllMustPass: predicates are ANDed, matching the +// query path's "AND"-joined WHERE clause. +func TestRowVisible_MultiplePredicates_AllMustPass(t *testing.T) { + t.Parallel() + perms := evalRowFilter(t, map[string]Filter{ + "tenant_id": {Eq: new("{{ jwt.tenant }}")}, + "amount": {Gt: new("100")}, + }, map[string]any{"tenant": "acme"}) + num := map[string]bool{"amount": true} + assert.True(t, perms.RowVisible(map[string]any{"tenant_id": "acme", "amount": float64(250)}, num)) + assert.False(t, perms.RowVisible(map[string]any{"tenant_id": "acme", "amount": float64(9)}, num), "amount fails") + assert.False(t, perms.RowVisible(map[string]any{"tenant_id": "globex", "amount": float64(250)}, num), "tenant fails") +} diff --git a/internal/stream/hub.go b/internal/stream/hub.go index e84821fa..934556f4 100644 --- a/internal/stream/hub.go +++ b/internal/stream/hub.go @@ -5,25 +5,29 @@ import ( "encoding/json" "sync" + "github.com/Wave-RF/WaveHouse/internal/discovery" "github.com/Wave-RF/WaveHouse/internal/ingest" "github.com/Wave-RF/WaveHouse/internal/policy" ) -// Hub fans live events out to SSE subscribers, projecting and serializing each -// event ONCE per (topic, role) instead of once per subscriber — the #294 lever. -// Subscribers register under (topic, role); Broadcast decodes the event once, -// applies each subscribed role's column policy once, builds one SSE frame per -// role, and fans it to every member of that role's Bucket. +// Hub fans live events out to SSE subscribers. Column projection is serialized ONCE +// per (topic, role) instead of once per subscriber — the #294/#353 lever — because +// column visibility depends solely on the role+table policy entry, never on JWT +// claims, so the projected frame is identical for every subscriber of a role. // -// The (topic, role) key is sufficient because the live path's only per-subscriber -// transform is column filtering, which depends solely on the role+table policy -// entry — never on JWT claims (claims feed only row-level WHERE/CHECK, which the -// stream path does not apply). See projectData. +// Row-level security is the exception: a role's row-filter (RLS predicate) is +// resolved against each subscriber's claims, so two subscribers of the same role can +// be entitled to different rows. For a role that carries a row-filter, Broadcast +// therefore keeps the shared column projection but evaluates row visibility PER +// subscriber (ResolvedPermissions.RowVisible) before delivering — closing the +// query/stream RLS drift in #319. Roles without a row-filter keep the pure +// once-per-role fast path unchanged. See projectColumns. type Hub struct { - mu sync.RWMutex - topics map[string]*topicRoutes - policy *policy.Store // nil ⇒ policy filtering not configured (legacy passthrough) - metric *Metrics // nil-safe + mu sync.RWMutex + topics map[string]*topicRoutes + policy *policy.Store // nil ⇒ policy filtering not configured (legacy passthrough) + registry *discovery.SchemaRegistry // nil ⇒ no column types; row-filter ordering compares lexicographically + metric *Metrics // nil-safe } // topicRoutes holds the per-role buckets subscribed to one topic. @@ -33,9 +37,11 @@ type topicRoutes struct { // NewHub builds an event hub. A nil policy store passes every event through // unfiltered (the unwired-tests case); a non-nil store whose Get returns nil is a -// total lockout (a deleted/absent policy denies everyone). metric may be nil. -func NewHub(policyStore *policy.Store, metric *Metrics) *Hub { - return &Hub{topics: make(map[string]*topicRoutes), policy: policyStore, metric: metric} +// total lockout (a deleted/absent policy denies everyone). A nil registry disables +// numeric-aware row-filter comparison (ordering predicates fall back to +// lexicographic); metric may be nil. +func NewHub(policyStore *policy.Store, registry *discovery.SchemaRegistry, metric *Metrics) *Hub { + return &Hub{topics: make(map[string]*topicRoutes), policy: policyStore, registry: registry, metric: metric} } // Add registers sub to receive events for (topic, role), creating the role bucket @@ -100,10 +106,12 @@ type roleBucket struct { bucket Bucket } -// Broadcast projects raw — a published EventMessage JSON delivered on topic — -// once per subscribed role and fans the finished SSE frame to that role's bucket. -// The expensive work (decode, evaluate, marshal) happens once per distinct role, -// not once per subscriber. +// Broadcast projects raw — a published EventMessage JSON delivered on topic — and +// fans the finished SSE frame to each subscribed role's bucket. The column +// projection (decode, evaluate, marshal) happens once per distinct role. For a role +// that carries a row-level-security filter, that shared frame is still delivered only +// to the subscribers whose claims admit this row (evaluated per subscriber); a role +// without a filter takes the pure once-per-role fast path. func (h *Hub) Broadcast(topic string, raw []byte) { // Snapshot the role->bucket set under the read lock; do the unmarshal / project // / serialize outside it. Skip the decode entirely when nobody is listening. @@ -125,13 +133,43 @@ func (h *Hub) Broadcast(topic string, raw []byte) { decoded := json.Unmarshal(raw, &evt) == nil && evt.TableName != "" p, filter := h.snapshotPolicy() + // Column types for numeric-aware row-filter comparison — resolved lazily at most + // once per event, only when some role actually carries a row-filter, and reused + // across every filtered role and subscriber. + var numericCols map[string]bool + numericResolved := false + for _, rb := range roleBuckets { - wire, ok := project(p, filter, rb.role, &evt, raw, decoded) + wire, perms, ok := projectColumns(p, filter, rb.role, &evt, raw, decoded) if !ok { continue // denied table / invalid payload for this role } frame := Frame{Kind: KindEvent, Data: wire} + + if !perms.HasRowFilter() { + // No row-level security for this role: one projection serves the whole + // bucket, regardless of per-subscriber claims (the #294/#353 fast path). + for _, sub := range rb.bucket.Snapshot() { + if !sub.Send(frame) { + h.metric.FrameDropped(KindEvent) + } + } + continue + } + + // Row-level security applies. The column projection is claims-independent and + // shared, but whether each subscriber may see THIS row depends on its claims, so + // evaluate visibility per subscriber. Predicates read the full event data (a + // filter may key on a column the role can't SELECT), not the projected columns. + if !numericResolved { + numericCols = h.numericCols(evt.TableName) + numericResolved = true + } for _, sub := range rb.bucket.Snapshot() { + subPerms := policy.Evaluate(p, rb.role, evt.TableName, "select", sub.claims) + if !subPerms.RowVisible(evt.Data, numericCols) { + continue // this row is filtered out for this subscriber + } if !sub.Send(frame) { h.metric.FrameDropped(KindEvent) } @@ -139,6 +177,26 @@ func (h *Hub) Broadcast(topic string, raw []byte) { } } +// numericCols maps each of the table's columns to whether its ClickHouse type is +// numeric, so the row-filter evaluator compares numeric columns numerically (9 < 100) +// rather than lexicographically. nil when no schema is available (unknown table, or a +// Hub built without a registry), in which case ordering predicates fall back to +// lexicographic comparison. +func (h *Hub) numericCols(table string) map[string]bool { + if h.registry == nil { + return nil + } + schema := h.registry.Get(table) + if schema == nil { + return nil + } + m := make(map[string]bool, len(schema.Columns)) + for _, c := range schema.Columns { + m[c.Name] = discovery.IsNumericType(c.Type) + } + return m +} + // snapshotPolicy returns the current policy and whether filtering is configured. // filter is false only when no store is wired (legacy passthrough); a wired store // returning a nil policy is a deliberate lockout that Evaluate denies. @@ -149,50 +207,60 @@ func (h *Hub) snapshotPolicy() (p *policy.Policy, filter bool) { return h.policy.Get(), true } -// ReplayFrame projects a single gap-fill event for one role into a ready-to-write -// replay frame, or ok=false to skip it (denied table / invalid payload). It is a -// method so replay shares the Hub's policy store with the live fan-out — the -// handler can't accidentally project replay against a different (or nil) policy. -// The per-connection replay path uses this; the live path uses Broadcast, which -// projects once per role instead of once per connection. -func (h *Hub) ReplayFrame(role string, raw []byte) (Frame, bool) { +// ReplayFrame projects a single gap-fill event for one role+claims into a +// ready-to-write replay frame, or ok=false to skip it (denied table, invalid +// payload, or a row the claims aren't entitled to see). It is a method so replay +// shares the Hub's policy store and schema registry with the live fan-out — the +// handler can't accidentally project replay against a different (or nil) policy. The +// per-connection replay path uses this; the live path uses Broadcast. Replay is +// already per-connection, so it evaluates row-level security against this +// connection's claims directly. +func (h *Hub) ReplayFrame(role string, claims map[string]any, raw []byte) (Frame, bool) { var evt ingest.EventMessage decoded := json.Unmarshal(raw, &evt) == nil && evt.TableName != "" p, filter := h.snapshotPolicy() - wire, ok := project(p, filter, role, &evt, raw, decoded) + wire, perms, ok := projectColumns(p, filter, role, &evt, raw, decoded) if !ok { return Frame{}, false } + if perms.HasRowFilter() { + subPerms := policy.Evaluate(p, role, evt.TableName, "select", claims) + if !subPerms.RowVisible(evt.Data, h.numericCols(evt.TableName)) { + return Frame{}, false // this row is filtered out for these claims + } + } return Frame{Kind: KindReplay, Data: wire}, true } -// project applies role/table column policy to a decoded EventMessage (or passes a -// non-EventMessage JSON payload through untouched), returning the SSE wire frame. -// ok=false means skip: the role can't read the table, or the payload is unusable. -// -// claims are intentionally not consulted: column visibility derives only from the -// role+table policy entry (AllowColumns/DenyColumns), so the projection is -// byte-identical for every subscriber of a (role, table) regardless of claims — -// which is exactly what lets one serialization serve the whole bucket. If row-level -// filtering is ever added to the stream path, this key (and projection) must take -// claims into account. -func project(p *policy.Policy, filter bool, role string, evt *ingest.EventMessage, raw []byte, decoded bool) (wire []byte, ok bool) { +// projectColumns applies role/table COLUMN policy to a decoded EventMessage (or +// passes a non-EventMessage JSON payload through untouched), returning the SSE wire +// frame and the resolved permissions. Column visibility derives only from the +// role+table policy entry (AllowColumns/DenyColumns), never from claims, so this +// frame is byte-identical for every subscriber of a (role, table) — the shared +// projection that lets one serialization serve a whole bucket. Row-level security is +// NOT applied here; the caller checks perms.RowVisible per subscriber (with that +// subscriber's claims) when perms.HasRowFilter(). ok=false means skip: the role can't +// read the table, or the payload is unusable. perms is nil for the legacy no-policy +// passthrough (which has no row-filter). +func projectColumns(p *policy.Policy, filter bool, role string, evt *ingest.EventMessage, raw []byte, decoded bool) (wire []byte, perms *policy.ResolvedPermissions, ok bool) { if !decoded { // Without a decoded EventMessage there's no table to evaluate policy against, // so fail closed whenever policy is configured: a malformed-but-valid-JSON // payload on ingest. must not bypass column filtering. Pass through // only when no policy store is wired at all (the legacy/test passthrough). if filter || !json.Valid(raw) { - return nil, false + return nil, nil, false } - return wireFrame("", raw), true + return wireFrame("", raw), nil, true } data := evt.Data if filter { - perms := policy.Evaluate(p, role, evt.TableName, "select", nil) + // Column allow/deny is claims-independent, so evaluate it once with nil claims; + // the caller re-evaluates the row-filter per subscriber against real claims. + perms = policy.Evaluate(p, role, evt.TableName, "select", nil) if !perms.Allowed { - return nil, false // role has no access to this table + return nil, nil, false // role has no access to this table } data = filterColumns(evt.Data, perms) } @@ -203,9 +271,9 @@ func project(p *policy.Policy, filter bool, role string, evt *ingest.EventMessag "data": data, }) if err != nil { - return nil, false + return nil, nil, false } - return wireFrame(evt.ReceivedTimestamp, payload), true + return wireFrame(evt.ReceivedTimestamp, payload), perms, true } // filterColumns returns a copy of data containing only columns the role may see. diff --git a/internal/stream/hub_test.go b/internal/stream/hub_test.go index 311587fe..48a4a983 100644 --- a/internal/stream/hub_test.go +++ b/internal/stream/hub_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/Wave-RF/WaveHouse/internal/discovery" "github.com/Wave-RF/WaveHouse/internal/ingest" "github.com/Wave-RF/WaveHouse/internal/policy" "github.com/stretchr/testify/assert" @@ -37,6 +38,17 @@ func recvFrame(t *testing.T, sub *Subscriber) Frame { } } +// assertNoFrame fails if sub has any frame buffered — used to prove a row-filtered +// subscriber received nothing for a row it isn't entitled to see. +func assertNoFrame(t *testing.T, sub *Subscriber) { + t.Helper() + select { + case f := <-sub.Frames(): + t.Fatalf("expected no frame, got %q", f.Data) + default: + } +} + // frameData parses the JSON object on the "data:" line of an SSE frame. func frameData(t *testing.T, f Frame) map[string]any { t.Helper() @@ -53,7 +65,7 @@ func frameData(t *testing.T, f Frame) map[string]any { func TestHub_ProjectsOncePerRole_FanOutToAllSubscribers(t *testing.T) { t.Parallel() - hub := NewHub(nil, nil) // nil store ⇒ passthrough, no filtering + hub := NewHub(nil, nil, nil) // nil store ⇒ passthrough, no filtering const topic = "ingest.clicks" a, b := NewSubscriber(), NewSubscriber() @@ -85,7 +97,7 @@ func TestHub_ProjectsPerRole_ColumnFilterAndDenial(t *testing.T) { }, }, } - hub := NewHub(policy.NewMemoryStore(p), nil) + hub := NewHub(policy.NewMemoryStore(p), nil, nil) const topic = "ingest.clicks" viewer := NewSubscriber() @@ -121,7 +133,7 @@ func TestHub_ProjectsPerRole_DistinctRolesGetDistinctFrames(t *testing.T) { }, }, } - hub := NewHub(policy.NewMemoryStore(p), nil) + hub := NewHub(policy.NewMemoryStore(p), nil, nil) const topic = "ingest.clicks" viewer, editor := NewSubscriber(), NewSubscriber() @@ -148,9 +160,132 @@ func TestHub_ProjectsPerRole_DistinctRolesGetDistinctFrames(t *testing.T) { assert.NotEqual(t, fv.Data, fe.Data, "distinct role projections produce distinct bytes") } +// rowFilterPolicy scopes role "viewer" to column "page" only, and to rows whose +// tenant_id equals the caller's {{ jwt.tenant }} claim. The filter keys on tenant_id +// — a column viewer may NOT select — so it also exercises the rule that row +// visibility is evaluated against the FULL event, then columns are projected. +func rowFilterPolicy() *policy.Policy { + return &policy.Policy{ + Tables: map[string]policy.TablePolicy{ + "clicks": { + Select: map[string]policy.RolePermissions{ + "viewer": { + AllowColumns: []string{"page"}, + Filter: map[string]policy.Filter{"tenant_id": {Eq: new("{{ jwt.tenant }}")}}, + }, + }, + }, + }, + } +} + +// TestHub_RowFilter_PerSubscriberIsolation is the #319 fix: two subscribers of the +// SAME role but different tenant claims each receive only their own tenant's rows +// over the live stream — the row-filter the query path applies is now applied here +// too, per subscriber. +func TestHub_RowFilter_PerSubscriberIsolation(t *testing.T) { + t.Parallel() + hub := NewHub(policy.NewMemoryStore(rowFilterPolicy()), nil, nil) + const topic = "ingest.clicks" + + acme := NewSubscriber() + acme.SetClaims(map[string]any{"tenant": "acme"}) + globex := NewSubscriber() + globex.SetClaims(map[string]any{"tenant": "globex"}) + hub.Add(topic, "viewer", acme) + hub.Add(topic, "viewer", globex) + + // An acme row reaches only the acme subscriber, projected to the allowed column. + hub.Broadcast(topic, rawEvent(t, "clicks", "2026-06-26T00:00:00Z", + map[string]any{"tenant_id": "acme", "page": "/a", "secret": "x"})) + inner := frameData(t, recvFrame(t, acme))["data"].(map[string]any) + assert.Equal(t, "/a", inner["page"]) + assert.NotContains(t, inner, "tenant_id", "the filtered column is not in viewer's projection") + assert.NotContains(t, inner, "secret", "denied column stripped") + assertNoFrame(t, globex) + + // A globex row reaches only the globex subscriber. + hub.Broadcast(topic, rawEvent(t, "clicks", "2026-06-26T00:00:01Z", + map[string]any{"tenant_id": "globex", "page": "/g"})) + assert.Equal(t, "/g", frameData(t, recvFrame(t, globex))["data"].(map[string]any)["page"]) + assertNoFrame(t, acme) +} + +// TestHub_RowFilter_MissingColumn_FailsClosed: an event that lacks the filtered +// column can't be proven visible, so it is withheld rather than leaked. +func TestHub_RowFilter_MissingColumn_FailsClosed(t *testing.T) { + t.Parallel() + hub := NewHub(policy.NewMemoryStore(rowFilterPolicy()), nil, nil) + const topic = "ingest.clicks" + + acme := NewSubscriber() + acme.SetClaims(map[string]any{"tenant": "acme"}) + hub.Add(topic, "viewer", acme) + + hub.Broadcast(topic, rawEvent(t, "clicks", "2026-06-26T00:00:00Z", + map[string]any{"page": "/a"})) // no tenant_id + assertNoFrame(t, acme) +} + +// TestHub_RowFilter_SharedProjectionAcrossSameClaims: the column projection is still +// serialized once per role and shared — two subscribers with the same (matching) +// claims receive the identical frame bytes. Only the visibility decision is +// per-subscriber, not the serialization. +func TestHub_RowFilter_SharedProjectionAcrossSameClaims(t *testing.T) { + t.Parallel() + hub := NewHub(policy.NewMemoryStore(rowFilterPolicy()), nil, nil) + const topic = "ingest.clicks" + + a, b := NewSubscriber(), NewSubscriber() + a.SetClaims(map[string]any{"tenant": "acme"}) + b.SetClaims(map[string]any{"tenant": "acme"}) + hub.Add(topic, "viewer", a) + hub.Add(topic, "viewer", b) + + hub.Broadcast(topic, rawEvent(t, "clicks", "2026-06-26T00:00:00Z", + map[string]any{"tenant_id": "acme", "page": "/a"})) + + fa, fb := recvFrame(t, a), recvFrame(t, b) + require.NotEmpty(t, fa.Data) + require.NotEmpty(t, fb.Data) + assert.Same(t, &fa.Data[0], &fb.Data[0], "one serialization shared across same-role subscribers") +} + +// TestHub_RowFilter_NumericOrdering_SchemaInformed drives the registry-backed path: +// with a numeric column type in the schema, an `amount > 100` filter compares +// numerically, so amount=9 is withheld (a lexicographic "9" > "100" comparison would +// have leaked it) and amount=250 is delivered. +func TestHub_RowFilter_NumericOrdering_SchemaInformed(t *testing.T) { + t.Parallel() + reg := discovery.NewSchemaRegistryFromMap([]*discovery.TableSchema{ + {Name: "clicks", Columns: []discovery.Column{ + {Name: "amount", Type: "UInt64"}, + {Name: "page", Type: "String"}, + }}, + }) + p := &policy.Policy{ + Tables: map[string]policy.TablePolicy{ + "clicks": {Select: map[string]policy.RolePermissions{ + "viewer": {Filter: map[string]policy.Filter{"amount": {Gt: new("100")}}}, + }}, + }, + } + hub := NewHub(policy.NewMemoryStore(p), reg, nil) + const topic = "ingest.clicks" + + sub := NewSubscriber() // constant filter value ⇒ no claims needed + hub.Add(topic, "viewer", sub) + + hub.Broadcast(topic, rawEvent(t, "clicks", "t1", map[string]any{"amount": float64(9), "page": "/a"})) + assertNoFrame(t, sub) // 9 is not numerically > 100 + + hub.Broadcast(topic, rawEvent(t, "clicks", "t2", map[string]any{"amount": float64(250), "page": "/b"})) + assert.Equal(t, float64(250), frameData(t, recvFrame(t, sub))["data"].(map[string]any)["amount"]) +} + func TestHub_TopicIsolation(t *testing.T) { t.Parallel() - hub := NewHub(nil, nil) + hub := NewHub(nil, nil, nil) clicks, views := NewSubscriber(), NewSubscriber() hub.Add("ingest.clicks", "public", clicks) hub.Add("ingest.views", "public", views) @@ -193,7 +328,7 @@ func TestHub_PassthroughAndFailClosed(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - hub := NewHub(tt.store, nil) + hub := NewHub(tt.store, nil, nil) const topic = "ingest.custom" sub := NewSubscriber() hub.Add(topic, "public", sub) @@ -214,7 +349,7 @@ func TestHub_PassthroughAndFailClosed(t *testing.T) { func TestHub_AddRemoveGCsBucketsAndTopics(t *testing.T) { t.Parallel() - hub := NewHub(nil, nil) + hub := NewHub(nil, nil, nil) const topic = "ingest.clicks" sub := NewSubscriber() @@ -235,7 +370,7 @@ func TestHub_AddRemoveGCsBucketsAndTopics(t *testing.T) { func TestHub_BroadcastNoSubscribers_NoOp(t *testing.T) { t.Parallel() - hub := NewHub(nil, nil) + hub := NewHub(nil, nil, nil) assert.NotPanics(t, func() { hub.Broadcast("ingest.nobody", rawEvent(t, "clicks", "t", map[string]any{"a": float64(1)})) }) @@ -252,7 +387,7 @@ func TestHub_SlowConsumerDropIncrementsMetric(t *testing.T) { otel.SetMeterProvider(savedMP) }) - hub := NewHub(nil, NewMetrics()) + hub := NewHub(nil, nil, NewMetrics()) const topic = "ingest.clicks" sub := newSubscriber(1) // cap-1: the second undrained broadcast drops hub.Add(topic, "public", sub) @@ -273,7 +408,7 @@ func TestHub_ReplayFrame(t *testing.T) { "clicks": {Select: map[string]policy.RolePermissions{"viewer": {AllowColumns: []string{"page"}}}}, }, } - hub := NewHub(policy.NewMemoryStore(p), nil) + hub := NewHub(policy.NewMemoryStore(p), nil, nil) raw := rawEvent(t, "clicks", "2026-06-26T00:00:00Z", map[string]any{"page": "/home", "secret": "x"}) tests := []struct { @@ -287,7 +422,7 @@ func TestHub_ReplayFrame(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - f, ok := hub.ReplayFrame(tt.role, raw) + f, ok := hub.ReplayFrame(tt.role, nil, raw) require.Equal(t, tt.want, ok) if !tt.want { return @@ -302,7 +437,7 @@ func TestHub_ReplayFrame(t *testing.T) { func TestHub_ConcurrentAddRemoveBroadcast_Race(t *testing.T) { t.Parallel() - hub := NewHub(nil, nil) + hub := NewHub(nil, nil, nil) const topic = "ingest.clicks" raw := rawEvent(t, "clicks", "t", map[string]any{"a": float64(1)}) diff --git a/internal/stream/subscriber.go b/internal/stream/subscriber.go index 253980e5..3a22b909 100644 --- a/internal/stream/subscriber.go +++ b/internal/stream/subscriber.go @@ -23,6 +23,13 @@ type Subscriber struct { out chan Frame // ready-to-write frames; see defaultSubscriberQueue bucket Bucket // set on Add so the keepalive wheel's Remove is O(1); nil until added + // claims are this connection's JWT claims, evaluated against a role's row-level + // security filter so the Hub can decide, per subscriber, whether each event row is + // visible (see Hub.Broadcast). Set once via SetClaims before Add, then read-only — + // so the fan-out goroutine reads it without synchronization. nil ⇒ no claims (a + // tokenless subscriber), which fails any claim-scoped row-filter closed. + claims map[string]any + // evict is closed (once) to ask the owning handler to disconnect a wedged slow // consumer; the handler selects on Evicted() and tears the stream down, after // which the client reconnects and gap-fills via Last-Event-ID. The seam is wired @@ -41,6 +48,12 @@ func newSubscriber(size int) *Subscriber { return &Subscriber{out: make(chan Frame, size), evict: make(chan struct{})} } +// SetClaims attaches this connection's JWT claims, which the Hub evaluates against a +// role's row-level-security filter to decide, per subscriber, whether each event row +// is visible. Call it before registering the subscriber with the Hub; the claims are +// then read-only for the subscriber's lifetime, so the fan-out reads them race-free. +func (s *Subscriber) SetClaims(claims map[string]any) { s.claims = claims } + // Frames is the queue of ready-to-write frames; the handler writes whatever // arrives here to the client verbatim. func (s *Subscriber) Frames() <-chan Frame { diff --git a/tests/integration/setup_test.go b/tests/integration/setup_test.go index bb3aecdb..348cabda 100644 --- a/tests/integration/setup_test.go +++ b/tests/integration/setup_test.go @@ -312,7 +312,7 @@ func buildServer(ch *chInstance, embeddedMQ *mq.EmbeddedNATS, registry *discover js := embeddedMQ.JetStream() policyStore := policy.NewMemoryStore(&policy.Policy{AdminRole: "admin"}) - streamHub := stream.NewHub(policyStore, nil) + streamHub := stream.NewHub(policyStore, registry, nil) deps := api.Dependencies{ Ingest: api.NewIngestHandler(registry, embeddedMQ, logger), From d534a251fb9fec9c91ec1669eb01690f7e5fdc15 Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 8 Jul 2026 08:44:17 -0400 Subject: [PATCH 2/9] Test case fixes --- docs/src/content/docs/access-control.mdx | 2 +- internal/stream/hub_test.go | 26 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/docs/src/content/docs/access-control.mdx b/docs/src/content/docs/access-control.mdx index 13664a15..faae1027 100644 --- a/docs/src/content/docs/access-control.mdx +++ b/docs/src/content/docs/access-control.mdx @@ -370,7 +370,7 @@ The same policy drives every data path, but not every field is meaningful on eve | ------- | -------- | -------- | | Structured read | `POST /v1/query?table={table}` | table+role `select` required, then `allow`/`deny_columns`, row `filter`, aggregation rules, and the per-role `max_rows` / `max_execution_time` / `max_rows_to_read` / `max_memory_usage` caps (over the [ClickHouse server-wide limits](/configuration#server-side-resource-limits)) | | Ingest (write) | `POST /v1/ingest?table={table}` | table+role `insert` required, then `allow`/`deny_columns` and `check` (enforced and auto-injected) | -| Live stream | `GET /v1/stream` | table+role `select` required (a table the role can't read is skipped), then denied columns are masked from each event | +| Live stream | `GET /v1/stream` | table+role `select` required (a table the role can't read is skipped), then denied columns are masked from each event and the role's row `filter` is applied per subscriber against their JWT claims (see caution below) | | Raw SQL | `POST /v1/admin/query` | `admin_role` only — no per-statement policy; the role gate is the entire authorization story | | Named pipe | `GET/POST /v1/pipes/{name}` | per-pipe `allowed_roles` (not the policy engine; see [Named Pipes](/pipes)). Resource limits come from ClickHouse's [server-wide settings](/configuration#server-side-resource-limits), not per-role policy caps | diff --git a/internal/stream/hub_test.go b/internal/stream/hub_test.go index 48a4a983..f07d92cf 100644 --- a/internal/stream/hub_test.go +++ b/internal/stream/hub_test.go @@ -435,6 +435,32 @@ func TestHub_ReplayFrame(t *testing.T) { } } +// TestHub_ReplayFrame_RowFilter exercises the row-filter branch of ReplayFrame: the +// #319 fix applies row-level security on the per-connection replay path too, so a +// gap-fill event is projected only when the connection's claims satisfy the filter. +func TestHub_ReplayFrame_RowFilter(t *testing.T) { + t.Parallel() + hub := NewHub(policy.NewMemoryStore(rowFilterPolicy()), nil, nil) + raw := rawEvent(t, "clicks", "2026-06-26T00:00:00Z", + map[string]any{"tenant_id": "acme", "page": "/a", "secret": "x"}) + + t.Run("matching claims replay the row, projected to allowed columns", func(t *testing.T) { + t.Parallel() + f, ok := hub.ReplayFrame("viewer", map[string]any{"tenant": "acme"}, raw) + require.True(t, ok) + assert.Equal(t, KindReplay, f.Kind) + inner := frameData(t, f)["data"].(map[string]any) + assert.Equal(t, "/a", inner["page"]) + assert.NotContains(t, inner, "secret", "denied column stripped on replay too") + }) + + t.Run("non-matching claims withhold the row", func(t *testing.T) { + t.Parallel() + _, ok := hub.ReplayFrame("viewer", map[string]any{"tenant": "globex"}, raw) + require.False(t, ok, "row must be withheld when claims don't satisfy the filter") + }) +} + func TestHub_ConcurrentAddRemoveBroadcast_Race(t *testing.T) { t.Parallel() hub := NewHub(nil, nil, nil) From af7afb9ca5cf5ea97a75ca40ea6594b88457095e Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 8 Jul 2026 10:38:06 -0400 Subject: [PATCH 3/9] perf(stream): resolve only the row filter per subscriber on SSE fan-out --- internal/policy/policy.go | 111 +++++++++++++++++++++++++++--------- internal/stream/hub.go | 4 +- internal/stream/hub_test.go | 43 +++++++++++++- 3 files changed, 126 insertions(+), 32 deletions(-) diff --git a/internal/policy/policy.go b/internal/policy/policy.go index c0f5a4f3..4c466775 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -108,8 +108,14 @@ func ResolveRole(p *Policy, role string) string { return p.DefaultRole } -// Evaluate resolves a policy for a given role, table, and operation against JWT claims. -func Evaluate(p *Policy, role, table, operation string, claims map[string]any) *ResolvedPermissions { +// resolveRolePerms navigates the policy to the RolePermissions governing +// (role, table, operation), applying role resolution, the admin bypass, and +// default-deny. It returns admin=true for the unconditional-bypass role (perms is the +// zero value — the caller grants full access) and ok=false for any denial (perms is +// the zero value). Evaluate (query path) and EvaluateRowFilter (stream path) share +// this one navigation so they can never disagree on who is authorized or which policy +// entry applies. +func resolveRolePerms(p *Policy, role, table, operation string) (perms RolePermissions, admin, ok bool) { // Map an empty/absent role to the configured default_role (no-op if none, // or if the policy is nil). A non-empty role is unchanged — roles never // inherit the default's permissions. @@ -125,20 +131,20 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * // over HTTP), so a nil policy falls through to the deny just below. Mirrors // RoleAllowed's admin short-circuit on the pipe path. if IsAdmin(p, role) { - return &ResolvedPermissions{Allowed: true} + return RolePermissions{}, true, false } // Beyond here the role is non-admin (non-empty only if it carried a concrete // role or matched a real default_role); any failure to find a matching entry // is a plain deny. if p == nil { - return &ResolvedPermissions{Allowed: false} + return RolePermissions{}, false, false } - tp, ok := p.Tables[table] - if !ok { + tp, found := p.Tables[table] + if !found { // No policy for this table — default deny. - return &ResolvedPermissions{Allowed: false} + return RolePermissions{}, false, false } var rolePerms map[string]RolePermissions @@ -148,11 +154,11 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * case "insert": rolePerms = tp.Insert default: - return &ResolvedPermissions{Allowed: false} + return RolePermissions{}, false, false } if rolePerms == nil { - return &ResolvedPermissions{Allowed: false} + return RolePermissions{}, false, false } // An empty/absent role must never match a role entry — a roleless request is @@ -161,9 +167,18 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * // role key in a policy therefore grants nothing: the policy-side twin of the // empty-AllowedRoles-entry footgun closed for pipes in #159. Matching is // exact — there is no "*" any-role wildcard. - perms, ok := RolePermissions{}, false - if role != "" { - perms, ok = rolePerms[role] + if role == "" { + return RolePermissions{}, false, false + } + perms, ok = rolePerms[role] + return perms, false, ok +} + +// Evaluate resolves a policy for a given role, table, and operation against JWT claims. +func Evaluate(p *Policy, role, table, operation string, claims map[string]any) *ResolvedPermissions { + perms, admin, ok := resolveRolePerms(p, role, table, operation) + if admin { + return &ResolvedPermissions{Allowed: true} } if !ok { return &ResolvedPermissions{Allowed: false} @@ -181,24 +196,16 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * MaxMemoryUsage: perms.MaxMemoryUsage, } - // Resolve filters into WHERE clause. A bind-unsafe filter column can't be - // emitted safely — a '?' in it would shift clickhouse-go's positional value - // binding, including this RLS filter's own bound value — so deny the role - // fail-closed rather than drop the predicate (which would widen row access) - // or emit a mis-bound query. validateRolePerms rejects such a policy at write - // time; this guards the query path as defense-in-depth (Evaluate does not - // re-validate the policy it is handed). + // Resolve the row-filter once into predicates, then render both read surfaces + // from that single source so they can't drift: the query path binds them into a + // SQL WHERE here; the stream path evaluates the same predicates in memory + // (ResolvedPermissions.RowVisible, via EvaluateRowFilter). resolveFilterPredicates + // fails closed on a bind-unsafe column (see there). if len(perms.Filter) > 0 { - for col := range perms.Filter { - if chsql.BindUnsafe(col) { - return &ResolvedPermissions{Allowed: false} - } + preds, deny := resolveFilterPredicates(perms.Filter, claims) + if deny { + return &ResolvedPermissions{Allowed: false} } - // Resolve the row-filter once into predicates, then render both read surfaces - // from that single source so they can't drift: the query path binds them into - // a SQL WHERE here; the stream path evaluates the same predicates in memory - // (ResolvedPermissions.RowVisible). - preds := resolvePredicates(perms.Filter, claims) resolved.rowFilter = preds clauses, params := predicatesToSQL(preds) if len(clauses) > 0 { @@ -225,6 +232,54 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * return resolved } +// EvaluateRowFilter resolves ONLY the row-level-security predicates governing +// (role, table, operation) for the given claims, returning a *ResolvedPermissions the +// caller evaluates with RowVisible. It is the stream fan-out's per-subscriber twin of +// Evaluate: it shares Evaluate's role resolution, admin bypass, default-deny, and +// bind-safe predicate resolution — so the two can never disagree on which rows a +// subscriber may see — but skips the column, aggregation, resource-cap, and SQL-WHERE +// work Evaluate does, none of which an in-memory row check needs. That keeps a +// high-fan-out filtered topic from allocating and discarding a full ResolvedPermissions +// per subscriber per event (see stream.BenchmarkBroadcast_RowFilteredFanout). +// +// The result carries Allowed and the resolved rowFilter only. Admin and roles without +// a filter yield no predicates (RowVisible admits every row); a denied role or a +// bind-unsafe filter yields Allowed:false. The stream calls this only after its column +// projection has already confirmed the role may read the table, so Allowed is +// invariantly true at that call site and RowVisible alone decides visibility. +func EvaluateRowFilter(p *Policy, role, table, operation string, claims map[string]any) *ResolvedPermissions { + perms, admin, ok := resolveRolePerms(p, role, table, operation) + if admin { + return &ResolvedPermissions{Allowed: true} + } + if !ok { + return &ResolvedPermissions{Allowed: false} + } + preds, deny := resolveFilterPredicates(perms.Filter, claims) + if deny { + return &ResolvedPermissions{Allowed: false} + } + return &ResolvedPermissions{Allowed: true, rowFilter: preds} +} + +// resolveFilterPredicates resolves a role's row-filter map into predicates, first +// failing closed (deny=true) if any filter column is bind-unsafe: a '?' in the column +// would shift clickhouse-go's positional value binding — including this filter's own +// bound value — so the role is denied rather than the predicate dropped (which would +// widen row access) or a mis-bound query emitted. validateRolePerms rejects such a +// policy at write time; this is defense-in-depth (the resolver does not re-validate the +// policy it is handed). Shared by Evaluate (query path, which then renders SQL via +// predicatesToSQL) and EvaluateRowFilter (stream path, which evaluates in memory), so +// both reject the same unsafe policy and resolve identical predicates. +func resolveFilterPredicates(filters map[string]Filter, claims map[string]any) (preds []resolvedPredicate, deny bool) { + for col := range filters { + if chsql.BindUnsafe(col) { + return nil, true + } + } + return resolvePredicates(filters, claims), false +} + // resolvedPredicate is one row-filter comparison with its claim templates already // resolved to concrete string values — the shared, render-agnostic form the query // path turns into SQL (predicatesToSQL) and the stream path evaluates in memory diff --git a/internal/stream/hub.go b/internal/stream/hub.go index 934556f4..11785134 100644 --- a/internal/stream/hub.go +++ b/internal/stream/hub.go @@ -166,7 +166,7 @@ func (h *Hub) Broadcast(topic string, raw []byte) { numericResolved = true } for _, sub := range rb.bucket.Snapshot() { - subPerms := policy.Evaluate(p, rb.role, evt.TableName, "select", sub.claims) + subPerms := policy.EvaluateRowFilter(p, rb.role, evt.TableName, "select", sub.claims) if !subPerms.RowVisible(evt.Data, numericCols) { continue // this row is filtered out for this subscriber } @@ -224,7 +224,7 @@ func (h *Hub) ReplayFrame(role string, claims map[string]any, raw []byte) (Frame return Frame{}, false } if perms.HasRowFilter() { - subPerms := policy.Evaluate(p, role, evt.TableName, "select", claims) + subPerms := policy.EvaluateRowFilter(p, role, evt.TableName, "select", claims) if !subPerms.RowVisible(evt.Data, h.numericCols(evt.TableName)) { return Frame{}, false // this row is filtered out for these claims } diff --git a/internal/stream/hub_test.go b/internal/stream/hub_test.go index f07d92cf..6cf219fc 100644 --- a/internal/stream/hub_test.go +++ b/internal/stream/hub_test.go @@ -3,6 +3,7 @@ package stream import ( "context" "encoding/json" + "fmt" "strings" "sync" "testing" @@ -18,8 +19,9 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata" ) -// rawEvent marshals an EventMessage the way the ingest path publishes it. -func rawEvent(t *testing.T, table, ts string, data map[string]any) []byte { +// rawEvent marshals an EventMessage the way the ingest path publishes it. It takes +// testing.TB so both tests (*testing.T) and benchmarks (*testing.B) can build events. +func rawEvent(t testing.TB, table, ts string, data map[string]any) []byte { t.Helper() raw, err := json.Marshal(ingest.EventMessage{TableName: table, ReceivedTimestamp: ts, Data: data}) require.NoError(t, err) @@ -504,6 +506,43 @@ func TestWireFrame(t *testing.T) { } } +// BenchmarkBroadcast_RowFilteredFanout measures one Broadcast on a row-filtered +// topic as the subscriber count grows. Every subscriber of a filtered role triggers +// a per-subscriber policy.Evaluate + RowVisible on each event (hub.go Broadcast), so +// this exercises the O(subscribers) allocation path CodeRabbit flagged on PR #381. +// It isolates that fan-out cost: the shared column projection is built once per +// event, and full outbound queues merely drop (a nil-metric no-op), so the +// allocs/op reported here are dominated by the per-subscriber evaluation. +// +// go test ./internal/stream/ -run '^$' -bench BenchmarkBroadcast_RowFilteredFanout -benchmem +func BenchmarkBroadcast_RowFilteredFanout(b *testing.B) { + const topic = "ingest.clicks" + raw := rawEvent(b, "clicks", "2026-06-26T00:00:00Z", + map[string]any{"tenant_id": "acme", "page": "/a", "secret": "x"}) + + for _, n := range []int{100, 1_000, 10_000} { + b.Run(fmt.Sprintf("subscribers=%d", n), func(b *testing.B) { + hub := NewHub(policy.NewMemoryStore(rowFilterPolicy()), nil, nil) + // Half the subscribers share the event's tenant (row visible), half don't + // (row withheld); either way each still pays the per-subscriber Evaluate, + // which is the cost under measurement. + for i := range n { + sub := NewSubscriber() + tenant := "acme" + if i%2 == 1 { + tenant = "globex" + } + sub.SetClaims(map[string]any{"tenant": tenant}) + hub.Add(topic, "viewer", sub) + } + b.ReportAllocs() + for b.Loop() { + hub.Broadcast(topic, raw) + } + }) + } +} + // sumByName totals all datapoints of an Int64 sum instrument across kinds. func sumByName(rm metricdata.ResourceMetrics, name string) int64 { for _, sm := range rm.ScopeMetrics { From 898748088dca1bede032b89260b752c5a830d498 Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 8 Jul 2026 12:31:16 -0400 Subject: [PATCH 4/9] perf(stream): compile row filter once per bucket, bind per subscriber --- go.mod | 2 +- internal/policy/policy.go | 275 ++++++++++++++++++++++-------- internal/policy/rowfilter.go | 63 +++++++ internal/policy/rowfilter_test.go | 125 ++++++++++++++ internal/stream/hub.go | 11 +- internal/stream/hub_test.go | 15 +- 6 files changed, 407 insertions(+), 84 deletions(-) diff --git a/go.mod b/go.mod index 7a1c7d32..0ea6370e 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/Wave-RF/WaveHouse -go 1.26.4 +go 1.26.5 tool ( github.com/Zxilly/go-size-analyzer/cmd/gsa diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 4c466775..2274198a 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -112,7 +112,7 @@ func ResolveRole(p *Policy, role string) string { // (role, table, operation), applying role resolution, the admin bypass, and // default-deny. It returns admin=true for the unconditional-bypass role (perms is the // zero value — the caller grants full access) and ok=false for any denial (perms is -// the zero value). Evaluate (query path) and EvaluateRowFilter (stream path) share +// the zero value). Evaluate (query path) and CompileRowFilter (stream path) share // this one navigation so they can never disagree on who is authorized or which policy // entry applies. func resolveRolePerms(p *Policy, role, table, operation string) (perms RolePermissions, admin, ok bool) { @@ -198,9 +198,9 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * // Resolve the row-filter once into predicates, then render both read surfaces // from that single source so they can't drift: the query path binds them into a - // SQL WHERE here; the stream path evaluates the same predicates in memory - // (ResolvedPermissions.RowVisible, via EvaluateRowFilter). resolveFilterPredicates - // fails closed on a bind-unsafe column (see there). + // SQL WHERE here; the stream path evaluates the same compiled predicates in memory + // (CompiledRowFilter.RowVisible). resolveFilterPredicates fails closed on a + // bind-unsafe column (see there). if len(perms.Filter) > 0 { preds, deny := resolveFilterPredicates(perms.Filter, claims) if deny { @@ -232,90 +232,234 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * return resolved } -// EvaluateRowFilter resolves ONLY the row-level-security predicates governing -// (role, table, operation) for the given claims, returning a *ResolvedPermissions the -// caller evaluates with RowVisible. It is the stream fan-out's per-subscriber twin of -// Evaluate: it shares Evaluate's role resolution, admin bypass, default-deny, and -// bind-safe predicate resolution — so the two can never disagree on which rows a -// subscriber may see — but skips the column, aggregation, resource-cap, and SQL-WHERE -// work Evaluate does, none of which an in-memory row check needs. That keeps a -// high-fan-out filtered topic from allocating and discarding a full ResolvedPermissions -// per subscriber per event (see stream.BenchmarkBroadcast_RowFilteredFanout). +// resolveFilterPredicates resolves a role's row-filter map into predicates for the +// query path, first failing closed (deny=true) if any filter column is bind-unsafe: a +// '?' in the column would shift clickhouse-go's positional value binding — including +// this filter's own bound value — so the role is denied rather than the predicate +// dropped (which would widen row access) or a mis-bound query emitted. validateRolePerms +// rejects such a policy at write time; this is defense-in-depth (the resolver does not +// re-validate the policy it is handed). The stream path applies the same bind-unsafe +// gate in CompileRowFilter, so both read surfaces reject the same unsafe policy. +func resolveFilterPredicates(filters map[string]Filter, claims map[string]any) (preds []resolvedPredicate, deny bool) { + if filterBindUnsafe(filters) { + return nil, true + } + return resolvePredicates(filters, claims), false +} + +// filterBindUnsafe reports whether any filter column can't be bound safely (see +// resolveFilterPredicates for why that denies the role). Claims-independent, so the +// stream can check it once per bucket in CompileRowFilter. +func filterBindUnsafe(filters map[string]Filter) bool { + for col := range filters { + if chsql.BindUnsafe(col) { + return true + } + } + return false +} + +// ----------------------------------------------------------------------------- +// Compiled row filter: claims-independent structure, per-subscriber claim binding. // -// The result carries Allowed and the resolved rowFilter only. Admin and roles without -// a filter yield no predicates (RowVisible admits every row); a denied role or a -// bind-unsafe filter yields Allowed:false. The stream calls this only after its column -// projection has already confirmed the role may read the table, so Allowed is -// invariantly true at that call site and RowVisible alone decides visibility. -func EvaluateRowFilter(p *Policy, role, table, operation string, claims map[string]any) *ResolvedPermissions { - perms, admin, ok := resolveRolePerms(p, role, table, operation) - if admin { - return &ResolvedPermissions{Allowed: true} +// The stream fan-out resolves a role's row-filter ONCE per bucket into compiled +// predicates (CompileRowFilter), then binds each subscriber's claims at evaluation +// time (CompiledRowFilter.RowVisible). It is the deferred-binding twin of +// resolvePredicates, which eagerly resolves for the query path's single claim set. +// The two must agree on every (filter, claims) pair — that parity is pinned by +// TestCompiledRowFilter_MatchesEvaluatePath. The optimization is that a whole +// {{ jwt.path }} value binds via a map walk (navigateClaims) and a constant binds to +// itself, so a filtered high-fan-out topic pays no per-subscriber regexp/predicate +// allocation. +// ----------------------------------------------------------------------------- + +type valueKind uint8 + +const ( + valConstant valueKind = iota // literal, no claim template + valClaim // exactly one {{ jwt.path }} — bound via navigateClaims + valTemplate // text with embedded {{ jwt.… }} — bound via resolveTemplate +) + +// compiledValue is a filter value with its claim-template structure classified once, +// so binding a subscriber's claims is a map walk (valClaim) or a no-op (valConstant) +// rather than a regexp pass. +type compiledValue struct { + kind valueKind + s string // valConstant literal, or valTemplate raw text + path []string // valClaim: the pre-split jwt claim path +} + +// compileScalarValue classifies an _eq/_neq/_gt/_lt value the way resolveTemplate reads +// it — a substring replace, with a whole-value (no surrounding text) {{ jwt.path }} +// short-circuited to a direct claim lookup. It matches on the UNtrimmed value so a +// space-padded ref stays a template, exactly as resolveTemplate would keep the spaces. +func compileScalarValue(v string) compiledValue { + if m := wholeClaimRe.FindStringSubmatch(v); m != nil { + return compiledValue{kind: valClaim, path: strings.Split(m[1], ".")} } - if !ok { - return &ResolvedPermissions{Allowed: false} + if claimTemplateRe.MatchString(v) { + return compiledValue{kind: valTemplate, s: v} } - preds, deny := resolveFilterPredicates(perms.Filter, claims) - if deny { - return &ResolvedPermissions{Allowed: false} + return compiledValue{kind: valConstant, s: v} +} + +// compileInValue classifies an _in value the way resolveInValues reads it: a +// TrimSpace'd whole {{ jwt.path }} is a claim list; anything else is a single +// (possibly templated) value. +func compileInValue(v string) compiledValue { + if m := wholeClaimRe.FindStringSubmatch(strings.TrimSpace(v)); m != nil { + return compiledValue{kind: valClaim, path: strings.Split(m[1], ".")} + } + if claimTemplateRe.MatchString(v) { + return compiledValue{kind: valTemplate, s: v} } - return &ResolvedPermissions{Allowed: true, rowFilter: preds} + return compiledValue{kind: valConstant, s: v} } -// resolveFilterPredicates resolves a role's row-filter map into predicates, first -// failing closed (deny=true) if any filter column is bind-unsafe: a '?' in the column -// would shift clickhouse-go's positional value binding — including this filter's own -// bound value — so the role is denied rather than the predicate dropped (which would -// widen row access) or a mis-bound query emitted. validateRolePerms rejects such a -// policy at write time; this is defense-in-depth (the resolver does not re-validate the -// policy it is handed). Shared by Evaluate (query path, which then renders SQL via -// predicatesToSQL) and EvaluateRowFilter (stream path, which evaluates in memory), so -// both reject the same unsafe policy and resolve identical predicates. -func resolveFilterPredicates(filters map[string]Filter, claims map[string]any) (preds []resolvedPredicate, deny bool) { - for col := range filters { - if chsql.BindUnsafe(col) { - return nil, true +// bindScalar resolves a scalar value against claims — the deferred twin of +// resolveTemplate. A whole-claim ref returns the claim's string form (no allocation +// when the claim is already a string; fmt.Sprint otherwise, matching resolveTemplate), +// and a missing claim resolves to "" exactly as resolveTemplate does. +func (cv compiledValue) bindScalar(claims map[string]any) string { + switch cv.kind { + case valClaim: + val := navigateClaims(claims, cv.path) + if val == nil { + return "" + } + if s, ok := val.(string); ok { + return s } + return fmt.Sprint(val) + case valTemplate: + return resolveTemplate(cv.s, claims) + case valConstant: + return cv.s + default: + return cv.s // unreachable: every valueKind has an explicit case above } - return resolvePredicates(filters, claims), false } -// resolvedPredicate is one row-filter comparison with its claim templates already -// resolved to concrete string values — the shared, render-agnostic form the query -// path turns into SQL (predicatesToSQL) and the stream path evaluates in memory -// (RowVisible). Op is one of "=", "!=", ">", "<", "in". Values holds one element -// for the scalar operators and zero-or-more for "in" (empty ⇒ matches no rows). -type resolvedPredicate struct { +// bindIn resolves an _in value against claims into the set of bound values (the +// deferred twin of resolveInValues). A claim list yields one bound value per element; +// any other value yields the single scalar-bound value. +func (cv compiledValue) bindIn(claims map[string]any) []string { + if cv.kind == valClaim { + switch v := navigateClaims(claims, cv.path).(type) { + case nil: + return nil + case []any: + out := make([]string, 0, len(v)) + for _, e := range v { + out = append(out, fmt.Sprint(e)) + } + return out + default: + return []string{fmt.Sprint(v)} + } + } + return []string{cv.bindScalar(claims)} +} + +// compiledPredicate is one row-filter comparison with its value structure compiled but +// the claim binding deferred to evaluation time. +type compiledPredicate struct { Column string Op string - Values []string + Value compiledValue } -// resolvePredicates resolves each filter's claim templates once into predicates. -// Both read surfaces derive from this single result so they can't drift; the -// operator order within a column (=, !=, >, <, in) mirrors the former inline SQL. -func resolvePredicates(filters map[string]Filter, claims map[string]any) []resolvedPredicate { - var preds []resolvedPredicate +// compilePredicates compiles a role's row-filter into per-subscriber-bindable +// predicates, mirroring resolvePredicates' column/operator expansion (and order) so the +// compiled and resolved paths produce the same predicate set. +func compilePredicates(filters map[string]Filter) []compiledPredicate { + var preds []compiledPredicate for col, f := range filters { if f.Eq != nil { - preds = append(preds, resolvedPredicate{col, "=", []string{resolveTemplate(*f.Eq, claims)}}) + preds = append(preds, compiledPredicate{col, "=", compileScalarValue(*f.Eq)}) } if f.Neq != nil { - preds = append(preds, resolvedPredicate{col, "!=", []string{resolveTemplate(*f.Neq, claims)}}) + preds = append(preds, compiledPredicate{col, "!=", compileScalarValue(*f.Neq)}) } if f.Gt != nil { - preds = append(preds, resolvedPredicate{col, ">", []string{resolveTemplate(*f.Gt, claims)}}) + preds = append(preds, compiledPredicate{col, ">", compileScalarValue(*f.Gt)}) } if f.Lt != nil { - preds = append(preds, resolvedPredicate{col, "<", []string{resolveTemplate(*f.Lt, claims)}}) + preds = append(preds, compiledPredicate{col, "<", compileScalarValue(*f.Lt)}) } if f.In != nil { - preds = append(preds, resolvedPredicate{col, "in", toStrings(resolveInValues(*f.In, claims))}) + preds = append(preds, compiledPredicate{col, "in", compileInValue(*f.In)}) } } return preds } +// CompiledRowFilter is a role/table's row-level-security filter compiled once +// (claims-independent) so the stream fan-out can reuse it across every subscriber in a +// bucket: resolve it per bucket with CompileRowFilter, then call RowVisible per +// subscriber with that subscriber's claims. It binds claims at evaluation time, so a +// filtered high-fan-out topic pays no per-subscriber predicate resolution — the +// deferred-binding counterpart to the query path's resolvePredicates (which binds a +// single claim set eagerly for SQL). +type CompiledRowFilter struct { + allowed bool + preds []compiledPredicate +} + +// CompileRowFilter compiles the row-filter governing (role, table, operation), +// claims-independently. It shares Evaluate's role resolution, admin bypass, and +// default-deny (via resolveRolePerms) and the same bind-unsafe gate, so it agrees with +// the query path on who is authorized and which policy is rejected. Compile once per +// role bucket; the per-subscriber cost is then only RowVisible. +func CompileRowFilter(p *Policy, role, table, operation string) *CompiledRowFilter { + perms, admin, ok := resolveRolePerms(p, role, table, operation) + if admin { + return &CompiledRowFilter{allowed: true} // admin: no predicates ⇒ every row visible + } + if !ok { + return &CompiledRowFilter{allowed: false} + } + if filterBindUnsafe(perms.Filter) { + return &CompiledRowFilter{allowed: false} // fail closed, as Evaluate does + } + return &CompiledRowFilter{allowed: true, preds: compilePredicates(perms.Filter)} +} + +// resolvedPredicate is one row-filter comparison with its claim templates already +// resolved to concrete string values — the shared, render-agnostic form the query +// path turns into SQL (predicatesToSQL) and the stream path evaluates in memory +// (RowVisible). Op is one of "=", "!=", ">", "<", "in". Values holds one element +// for the scalar operators and zero-or-more for "in" (empty ⇒ matches no rows). +type resolvedPredicate struct { + Column string + Op string + Values []string +} + +// resolvePredicates resolves each filter's claim templates into predicates for the +// query path. It compiles the filter — the single, claims-independent source the stream +// path also uses (CompileRowFilter) — then binds this request's claims, so the two read +// surfaces derive from one place and can't drift. Operator order within a column +// (=, !=, >, <, in) mirrors compilePredicates and the former inline SQL. +func resolvePredicates(filters map[string]Filter, claims map[string]any) []resolvedPredicate { + var preds []resolvedPredicate + for _, cp := range compilePredicates(filters) { + preds = append(preds, cp.resolve(claims)) + } + return preds +} + +// resolve binds a compiled predicate's claim value(s) into a resolvedPredicate — the +// eager, query-path counterpart to compiledPredicate.visible (which the stream +// evaluates against the row in memory instead of materializing this form). +func (cp compiledPredicate) resolve(claims map[string]any) resolvedPredicate { + if cp.Op == "in" { + return resolvedPredicate{cp.Column, "in", cp.Value.bindIn(claims)} + } + return resolvedPredicate{cp.Column, cp.Op, []string{cp.Value.bindScalar(claims)}} +} + // predicatesToSQL renders resolved predicates into WHERE clauses and bound params. func predicatesToSQL(preds []resolvedPredicate) ([]string, []any) { var clauses []string @@ -353,19 +497,6 @@ func resolveFilters(filters map[string]Filter, claims map[string]any) ([]string, return predicatesToSQL(resolvePredicates(filters, claims)) } -// toStrings normalizes resolveInValues' []any (already stringified elements) to the -// []string a resolvedPredicate carries. -func toStrings(vals []any) []string { - if len(vals) == 0 { - return nil - } - out := make([]string, len(vals)) - for i, v := range vals { - out[i] = fmt.Sprint(v) - } - return out -} - // resolveTemplate resolves {{ jwt.claim.path }} templates against JWT claims. // If a claim path cannot be resolved, the template placeholder is replaced with // an empty string to prevent "" from leaking into SQL filters. diff --git a/internal/policy/rowfilter.go b/internal/policy/rowfilter.go index efaf2f05..12983ffe 100644 --- a/internal/policy/rowfilter.go +++ b/internal/policy/rowfilter.go @@ -75,6 +75,69 @@ func (pred resolvedPredicate) matches(row map[string]any, numeric bool) bool { } } +// HasRowFilter reports whether the compiled filter carries any predicate. A nil +// receiver, or an admin / unfiltered role, has none — the compiled twin of +// ResolvedPermissions.HasRowFilter. +func (c *CompiledRowFilter) HasRowFilter() bool { + return c != nil && len(c.preds) > 0 +} + +// RowVisible reports whether row satisfies every compiled predicate for the given +// claims — the per-subscriber, deferred-binding twin of ResolvedPermissions.RowVisible, +// and it must agree with it on every decision. A nil receiver (no policy applies) +// admits every row; a denied or bind-unsafe filter admits none (fail closed). +// numericCols is as in ResolvedPermissions.RowVisible. +func (c *CompiledRowFilter) RowVisible(row, claims map[string]any, numericCols map[string]bool) bool { + if c == nil { + return true + } + if !c.allowed { + return false + } + for _, pred := range c.preds { + if !pred.visible(row, claims, numericCols[pred.Column]) { + return false + } + } + return true +} + +// visible evaluates one compiled predicate against the row for the given claims. It +// binds the claim value at call time, then reuses compareScalar — the same comparison +// core as resolvedPredicate.matches — so the compiled and resolved paths can't drift on +// how values compare. Fails closed on an absent or uncomparable value, exactly as +// matches does. +func (cp compiledPredicate) visible(row, claims map[string]any, numeric bool) bool { + raw, ok := row[cp.Column] + if !ok { + return false // column not in the event ⇒ can't prove the row is allowed + } + if cp.Op == "in" { + for _, v := range cp.Value.bindIn(claims) { + if c, ok := compareScalar(raw, v, numeric); ok && c == 0 { + return true + } + } + return false + } + c, ok := compareScalar(raw, cp.Value.bindScalar(claims), numeric) + if !ok { + return false + } + switch cp.Op { + case "=": + return c == 0 + case "!=": + return c != 0 + case ">": + return c > 0 + case "<": + return c < 0 + default: + return false + } +} + // compareScalar compares an event value against a resolved filter value, returning // -1/0/+1 and ok=false when the comparison can't be made (a non-scalar event value, // or a numeric comparison whose operands don't parse as numbers). numeric selects diff --git a/internal/policy/rowfilter_test.go b/internal/policy/rowfilter_test.go index 5b3b39a8..10707448 100644 --- a/internal/policy/rowfilter_test.go +++ b/internal/policy/rowfilter_test.go @@ -1,9 +1,11 @@ package policy import ( + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // evalRowFilter builds a one-role/one-table policy carrying filter and returns the @@ -105,3 +107,126 @@ func TestRowVisible_MultiplePredicates_AllMustPass(t *testing.T) { assert.False(t, perms.RowVisible(map[string]any{"tenant_id": "acme", "amount": float64(9)}, num), "amount fails") assert.False(t, perms.RowVisible(map[string]any{"tenant_id": "globex", "amount": float64(250)}, num), "tenant fails") } + +// TestCompiledRowFilter_MatchesEvaluatePath pins the parity the compiled per-subscriber +// stream path depends on: for every (filter, claims, row) below, CompileRowFilter + +// RowVisible must return exactly what the query path (Evaluate → resolved predicates → +// matches) returns. Were the two to diverge, a subscriber could see rows the query path +// hides (or vice versa) — the +// row-level-security bug this path exists to prevent. The matrix exercises every operator, +// constant / whole-claim / mixed-template / space-padded / nested-path values, and +// missing columns, empty claim sets, and numeric-vs-lexicographic comparison. +func TestCompiledRowFilter_MatchesEvaluatePath(t *testing.T) { + t.Parallel() + filters := []map[string]Filter{ + {"tenant_id": {Eq: new("{{ jwt.tenant }}")}}, + {"tenant_id": {Neq: new("{{ jwt.tenant }}")}}, + {"amount": {Gt: new("100")}}, + {"amount": {Lt: new("{{ jwt.cap }}")}}, + {"status": {Eq: new("active")}}, // constant + {"region": {In: new("{{ jwt.regions }}")}}, // claim list + {"tag": {In: new("{{ jwt.tag }}")}}, // claim scalar + {"kind": {In: new("published")}}, // constant in-set + {"label": {Eq: new("v-{{ jwt.tier }}")}}, // mixed template + {"tenant_id": {Eq: new(" {{ jwt.tenant }} ")}}, // space-padded ⇒ stays a template + {"tenant_id": {Eq: new("{{ jwt.tenant }}"), Neq: new("blocked")}}, // two predicates on one column + {"org": {Eq: new("{{ jwt.org.id }}")}}, // nested claim path + } + claimsets := []map[string]any{ + nil, + {"tenant": "acme"}, + {"tenant": "acme", "cap": float64(500), "regions": []any{"us", "eu"}, "tag": "x", "tier": "pro", "org": map[string]any{"id": "o1"}}, + {"tenant": ""}, + {"tenant": "acme", "regions": []any{}}, // empty claim list + } + rows := []map[string]any{ + {"tenant_id": "acme", "amount": float64(250), "status": "active", "region": "us", "tag": "x", "kind": "published", "label": "v-pro", "org": "o1"}, + {"tenant_id": "globex", "amount": float64(9), "status": "off", "region": "apac", "tag": "y", "kind": "draft", "label": "v-free", "org": "o2"}, + {"amount": float64(500)}, // several columns missing ⇒ fail closed + {"tenant_id": "", "amount": "NaN", "org": "o1"}, + } + numeric := map[string]bool{"amount": true} + + for fi, f := range filters { + p := &Policy{Tables: map[string]TablePolicy{ + "t": {Select: map[string]RolePermissions{"viewer": {Filter: f}}}, + }} + compiled := CompileRowFilter(p, "viewer", "t", "select") + assert.Equalf(t, Evaluate(p, "viewer", "t", "select", nil).HasRowFilter(), compiled.HasRowFilter(), + "HasRowFilter parity, filter#%d", fi) + for _, claims := range claimsets { + resolved := Evaluate(p, "viewer", "t", "select", claims) + for ri, row := range rows { + want := resolved.RowVisible(row, numeric) + got := compiled.RowVisible(row, claims, numeric) + assert.Equalf(t, want, got, + "filter#%d claims=%v row#%d — compiled=%v resolved=%v", fi, claims, ri, got, want) + } + } + } +} + +// FuzzCompiledRowFilterParity is the property form of the matrix test above: for ANY +// operator, filter value, claim, and row the fuzzer synthesizes, the compiled +// per-subscriber stream path (CompileRowFilter + RowVisible) must agree with the query +// path (Evaluate + resolved predicates). The fuzzer probes the template-parsing edges +// where a divergence would hide — stray "{{", "jwt.", nested dots, whitespace, partial +// templates — that a hand-written matrix can't enumerate. The seed corpus also runs as a +// plain regression test under `go test` (no -fuzz needed). The column is fixed to a +// bind-safe name so the role always resolves as allowed; RowVisible parity is defined +// only for an allowed role (a denied role never reaches the per-subscriber check). +func FuzzCompiledRowFilterParity(f *testing.F) { + f.Add(uint8(0), "{{ jwt.x }}", "acme", "acme", true, false, false) // eq claim-ref, match + f.Add(uint8(1), " {{ jwt.x }} ", "acme", "acme", true, false, false) // space-padded ⇒ template + f.Add(uint8(2), "100", "", "250", true, true, false) // gt numeric constant + f.Add(uint8(3), "{{ jwt.x }}", "500", "250", true, true, false) // lt numeric claim + f.Add(uint8(4), "{{ jwt.x }}", "a,b,c", "b", true, false, true) // in claim-list + f.Add(uint8(0), "v-{{ jwt.x }}", "pro", "v-pro", true, false, false) // mixed template + f.Add(uint8(0), "{{ jwt.x }}", "acme", "acme", false, false, false) // column absent ⇒ fail closed + f.Add(uint8(0), "{{ jwt.a.b }}", "z", "z", true, false, false) // nested claim path + + f.Fuzz(func(t *testing.T, opSel uint8, filterVal, claimVal, rowVal string, colPresent, numeric, claimList bool) { + var filter Filter + switch opSel % 5 { + case 0: + filter.Eq = &filterVal + case 1: + filter.Neq = &filterVal + case 2: + filter.Gt = &filterVal + case 3: + filter.Lt = &filterVal + case 4: + filter.In = &filterVal + } + p := &Policy{Tables: map[string]TablePolicy{ + "t": {Select: map[string]RolePermissions{"r": {Filter: map[string]Filter{"col": filter}}}}, + }} + + var claim any = claimVal + if claimList { // exercise the _in []any path, not just a scalar claim + parts := strings.Split(claimVal, ",") + list := make([]any, len(parts)) + for i, s := range parts { + list[i] = s + } + claim = list + } + claims := map[string]any{"x": claim} + row := map[string]any{} + if colPresent { + row["col"] = rowVal + } + nc := map[string]bool{} + if numeric { + nc["col"] = true + } + + compiled := CompileRowFilter(p, "r", "t", "select") + resolved := Evaluate(p, "r", "t", "select", claims) + require.True(t, resolved.Allowed, "fixed bind-safe setup must resolve as allowed") + require.Equalf(t, resolved.RowVisible(row, nc), compiled.RowVisible(row, claims, nc), + "parity broke: op=%d filter=%q claim=%v(list=%v) row=%q(present=%v) numeric=%v", + opSel%5, filterVal, claim, claimList, rowVal, colPresent, numeric) + }) +} diff --git a/internal/stream/hub.go b/internal/stream/hub.go index 11785134..eae6da8c 100644 --- a/internal/stream/hub.go +++ b/internal/stream/hub.go @@ -161,13 +161,16 @@ func (h *Hub) Broadcast(topic string, raw []byte) { // shared, but whether each subscriber may see THIS row depends on its claims, so // evaluate visibility per subscriber. Predicates read the full event data (a // filter may key on a column the role can't SELECT), not the projected columns. + // The filter compiles once per bucket (claims-independent); only the claim + // binding inside RowVisible is per subscriber, so a filtered high-fan-out topic + // pays no per-subscriber predicate resolution. if !numericResolved { numericCols = h.numericCols(evt.TableName) numericResolved = true } + compiled := policy.CompileRowFilter(p, rb.role, evt.TableName, "select") for _, sub := range rb.bucket.Snapshot() { - subPerms := policy.EvaluateRowFilter(p, rb.role, evt.TableName, "select", sub.claims) - if !subPerms.RowVisible(evt.Data, numericCols) { + if !compiled.RowVisible(evt.Data, sub.claims, numericCols) { continue // this row is filtered out for this subscriber } if !sub.Send(frame) { @@ -224,8 +227,8 @@ func (h *Hub) ReplayFrame(role string, claims map[string]any, raw []byte) (Frame return Frame{}, false } if perms.HasRowFilter() { - subPerms := policy.EvaluateRowFilter(p, role, evt.TableName, "select", claims) - if !subPerms.RowVisible(evt.Data, h.numericCols(evt.TableName)) { + compiled := policy.CompileRowFilter(p, role, evt.TableName, "select") + if !compiled.RowVisible(evt.Data, claims, h.numericCols(evt.TableName)) { return Frame{}, false // this row is filtered out for these claims } } diff --git a/internal/stream/hub_test.go b/internal/stream/hub_test.go index 6cf219fc..1a75683d 100644 --- a/internal/stream/hub_test.go +++ b/internal/stream/hub_test.go @@ -506,13 +506,14 @@ func TestWireFrame(t *testing.T) { } } -// BenchmarkBroadcast_RowFilteredFanout measures one Broadcast on a row-filtered -// topic as the subscriber count grows. Every subscriber of a filtered role triggers -// a per-subscriber policy.Evaluate + RowVisible on each event (hub.go Broadcast), so -// this exercises the O(subscribers) allocation path CodeRabbit flagged on PR #381. -// It isolates that fan-out cost: the shared column projection is built once per -// event, and full outbound queues merely drop (a nil-metric no-op), so the -// allocs/op reported here are dominated by the per-subscriber evaluation. +// BenchmarkBroadcast_RowFilteredFanout measures one Broadcast on a row-filtered topic +// as the subscriber count grows. The filter compiles once per bucket +// (policy.CompileRowFilter); each subscriber then pays only a claim-bound RowVisible on +// the full event, so allocations stay flat regardless of fan-out. This is the benchmark +// behind the O(subscribers) → O(1) allocation work on PR #381 — run it before/after a +// fan-out change to catch a regression back to per-subscriber resolution. It isolates +// the fan-out cost: the shared column projection is built once per event, and full +// outbound queues merely drop (a nil-metric no-op). // // go test ./internal/stream/ -run '^$' -bench BenchmarkBroadcast_RowFilteredFanout -benchmem func BenchmarkBroadcast_RowFilteredFanout(b *testing.B) { From 0e0c55a2d555b4f2006c8e9465872cf526b1e217 Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 8 Jul 2026 12:53:49 -0400 Subject: [PATCH 5/9] additional e2e test changes for coverage --- tests/e2e/sdk/streaming.test.ts | 56 +++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/tests/e2e/sdk/streaming.test.ts b/tests/e2e/sdk/streaming.test.ts index c6b4a5ea..a43d0792 100644 --- a/tests/e2e/sdk/streaming.test.ts +++ b/tests/e2e/sdk/streaming.test.ts @@ -1,6 +1,6 @@ import type { Policy } from "@wavehouse/sdk"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { adminClient, dataClient, publicClient, testId, waitForCondition } from "./helpers.js"; +import { adminClient, authClient, dataClient, publicClient, testId, waitForCondition } from "./helpers.js"; import { suiteTables } from "./tables.js"; describe("Streaming", () => { @@ -18,10 +18,18 @@ describe("Streaming", () => { const publicPolicy = structuredClone(baselinePolicy); publicPolicy.default_role = "anon"; - // Explicitly allow the 'anon' role to SELECT (stream) from this suite's tables + // Explicitly allow the 'anon' role to SELECT (stream) from this suite's tables. + // 'scoped' additionally carries a per-subscriber row filter — streamed rows are + // limited to the caller's own country claim — so the SSE fan-out exercises the + // row-level-security path (CompileRowFilter → RowVisible) end to end, not just + // column projection. publicPolicy.tables[T.clicks].select = { ...(publicPolicy.tables[T.clicks].select || {}), anon: { allow_columns: ["*"] }, + scoped: { + allow_columns: ["*"], + filter: { country: { _eq: "{{ jwt.country }}" } }, + }, }; publicPolicy.tables[T.events].select = { ...(publicPolicy.tables[T.events].select || {}), @@ -115,5 +123,49 @@ describe("Streaming", () => { stream.close(); } }); + + it("applies the role's row filter per subscriber (row-level scoping)", async () => { + // Two subscribers, same 'scoped' role but different country claims. The role's + // filter (country = {{ jwt.country }}) must be evaluated per subscriber against + // the full event, so each sees only its own country's rows over the live stream — + // the #319 row-level-security guarantee, exercised end to end on SSE. + const inserter = dataClient(); // viewer may insert into this suite's tables + const usClient = authClient("scoped", { country: "US" }); + const caClient = authClient("scoped", { country: "CA" }); + const usEvents: any[] = []; + const caEvents: any[] = []; + const usId = testId(); + const caId = testId(); + + const usStream = usClient.from(T.clicks).stream(); + const caStream = caClient.from(T.clicks).stream(); + let unsubUs: (() => void) | undefined; + let unsubCa: (() => void) | undefined; + try { + unsubUs = usStream.subscribe({ next: (e) => usEvents.push(e), error: (err) => console.error("US SSE error:", err) }); + unsubCa = caStream.subscribe({ next: (e) => caEvents.push(e), error: (err) => console.error("CA SSE error:", err) }); + await usStream.connected(20_000); + await caStream.connected(20_000); + + // One row per country; the filter must route each to only its matching subscriber. + const base = { page: "/scoped", user_id: "u", session_id: "s", duration_ms: 1 }; + await inserter.from(T.clicks).insert({ ...base, event_id: usId, country: "US" }); + await inserter.from(T.clicks).insert({ ...base, event_id: caId, country: "CA" }); + + // Each subscriber receives its own country's row. Waiting for BOTH positives + // proves both rows were broadcast, so the cross-absence checks below are real + // (a row filtered out at the source can never arrive later). + await waitForCondition(() => usEvents.some((e) => e.data?.event_id === usId), 10_000); + await waitForCondition(() => caEvents.some((e) => e.data?.event_id === caId), 10_000); + + expect(usEvents.some((e) => e.data?.event_id === caId)).toBe(false); + expect(caEvents.some((e) => e.data?.event_id === usId)).toBe(false); + } finally { + if (unsubUs) unsubUs(); + if (unsubCa) unsubCa(); + usStream.close(); + caStream.close(); + } + }); }); }); From dc7e902392de72ff4faba19bbc74f31ccb979b3f Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 8 Jul 2026 13:35:41 -0400 Subject: [PATCH 6/9] make fix --- tests/e2e/sdk/streaming.test.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/e2e/sdk/streaming.test.ts b/tests/e2e/sdk/streaming.test.ts index a43d0792..1dc58e0f 100644 --- a/tests/e2e/sdk/streaming.test.ts +++ b/tests/e2e/sdk/streaming.test.ts @@ -1,6 +1,13 @@ import type { Policy } from "@wavehouse/sdk"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { adminClient, authClient, dataClient, publicClient, testId, waitForCondition } from "./helpers.js"; +import { + adminClient, + authClient, + dataClient, + publicClient, + testId, + waitForCondition, +} from "./helpers.js"; import { suiteTables } from "./tables.js"; describe("Streaming", () => { @@ -142,8 +149,14 @@ describe("Streaming", () => { let unsubUs: (() => void) | undefined; let unsubCa: (() => void) | undefined; try { - unsubUs = usStream.subscribe({ next: (e) => usEvents.push(e), error: (err) => console.error("US SSE error:", err) }); - unsubCa = caStream.subscribe({ next: (e) => caEvents.push(e), error: (err) => console.error("CA SSE error:", err) }); + unsubUs = usStream.subscribe({ + next: (e) => usEvents.push(e), + error: (err) => console.error("US SSE error:", err), + }); + unsubCa = caStream.subscribe({ + next: (e) => caEvents.push(e), + error: (err) => console.error("CA SSE error:", err), + }); await usStream.connected(20_000); await caStream.connected(20_000); From 0a756f8089ba902bf324d717b74599e2206e1937 Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 8 Jul 2026 14:09:48 -0400 Subject: [PATCH 7/9] updated comment --- internal/stream/hub_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/stream/hub_test.go b/internal/stream/hub_test.go index 1a75683d..130b1765 100644 --- a/internal/stream/hub_test.go +++ b/internal/stream/hub_test.go @@ -525,8 +525,9 @@ func BenchmarkBroadcast_RowFilteredFanout(b *testing.B) { b.Run(fmt.Sprintf("subscribers=%d", n), func(b *testing.B) { hub := NewHub(policy.NewMemoryStore(rowFilterPolicy()), nil, nil) // Half the subscribers share the event's tenant (row visible), half don't - // (row withheld); either way each still pays the per-subscriber Evaluate, - // which is the cost under measurement. + // (row withheld); either way each pays the per-subscriber RowVisible check + // against the once-per-bucket compiled filter, which is the cost under + // measurement. for i := range n { sub := NewSubscriber() tenant := "acme" From dd38086c526407e2830cd68a44e274e3d77d4c5f Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 8 Jul 2026 15:38:43 -0400 Subject: [PATCH 8/9] fix(policy): fail closed on NaN and compare float64-equal row-filter ties at full precision --- CHANGELOG.md | 2 +- docs/src/content/docs/access-control.mdx | 2 +- docs/src/content/docs/architecture.md | 2 +- internal/policy/rowfilter.go | 27 +++++++++++++++++-- internal/policy/rowfilter_test.go | 34 ++++++++++++++++++++++++ internal/stream/hub.go | 7 ++--- 6 files changed, 66 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index faf59ded..a41fc54d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security -- **Live SSE streams now apply a role's row-`filter` per subscriber, closing a query/stream row-level-security drift** (`internal/stream/hub.go`, `internal/stream/subscriber.go`, `internal/policy/policy.go`, `internal/policy/rowfilter.go` (new), `internal/discovery/validation.go`, `internal/api/stream.go`, `cmd/wavehouse/main.go`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/architecture.md`, plus tests in `internal/policy/rowfilter_test.go` (new), `internal/discovery/validation_test.go`, `internal/stream/hub_test.go`): closes #319. The SSE delivery path stripped denied columns but never applied a role's row-level `filter` predicate, so a subscriber received rows the structured-query path would have filtered out for that same role — a data-exposure on the streaming surface for any table that combines a row-policy with a shared or role-scoped stream (harmless on the public Stats table today, which carries no restrictive row-policy, but real for any private/PII table fronted by a stream). The row-filter is now resolved once into predicates that feed **both** read surfaces — the query path renders them to SQL, the stream evaluates them in memory (`ResolvedPermissions.RowVisible`) — so the two can't drift (the row-level analogue of the shared `IsColumnAllowed` decision from #223). Because a row-filter resolves against each subscriber's JWT claims, the #294/#353 once-per-role projection is now claims-aware: a role **without** a filter keeps the pure once-per-role fast path unchanged (zero regression on the public stream), while a role **with** a filter keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (evaluated against the full event, so a filter may key on a column the role can't select). Equality and set predicates (`_eq`/`_neq`/`_in` — the tenant/user scoping row-security actually uses) are enforced exactly; ordering predicates (`_gt`/`_lt`) are schema-informed — numeric columns compare numerically (matching ClickHouse), other types lexicographically — which is best-effort for exotic types stored as numbers (a documented boundary in access-control.mdx), with every ambiguous or uncomparable value failing closed (the row is withheld). Replay (gap-fill) applies the same per-connection row check. The resource limits (`max_rows`, `max_execution_time`, …) remain a query-path property and are still **not** applied to the stream (a separate, documented boundary). Supersedes the "stream path applies no row-level filter" invariant noted in the #294/#353 Changed entry below. +- **Live SSE streams now apply a role's row-`filter` per subscriber, closing a query/stream row-level-security drift** (`internal/stream/hub.go`, `internal/stream/subscriber.go`, `internal/policy/policy.go`, `internal/policy/rowfilter.go` (new), `internal/discovery/validation.go`, `internal/api/stream.go`, `cmd/wavehouse/main.go`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/architecture.md`, plus tests in `internal/policy/rowfilter_test.go` (new), `internal/discovery/validation_test.go`, `internal/stream/hub_test.go`): closes #319. The SSE delivery path stripped denied columns but never applied a role's row-level `filter` predicate, so a subscriber received rows the structured-query path would have filtered out for that same role — a data-exposure on the streaming surface for any table that combines a row-policy with a shared or role-scoped stream (harmless on the public Stats table today, which carries no restrictive row-policy, but real for any private/PII table fronted by a stream). The row-filter is now resolved once into predicates that feed **both** read surfaces — the query path renders them to SQL, the stream evaluates them in memory (`policy.CompileRowFilter` → `CompiledRowFilter.RowVisible`, compiled once per role bucket and claim-bound per subscriber) — so the two can't drift (the row-level analogue of the shared `IsColumnAllowed` decision from #223). Because a row-filter resolves against each subscriber's JWT claims, the #294/#353 once-per-role projection is now claims-aware: a role **without** a filter keeps the pure once-per-role fast path unchanged (zero regression on the public stream), while a role **with** a filter keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (evaluated against the full event, so a filter may key on a column the role can't select). Equality and set predicates (`_eq`/`_neq`/`_in` — the tenant/user scoping row-security actually uses) are enforced exactly — numeric ties beyond `float64` precision resolve at arbitrary precision (`math/big`), so the string-encoded 64-bit IDs ingest accepts to survive JS precision loss never falsely collide, and a `NaN` operand fails closed rather than comparing equal to everything — with one representation caveat: a `Bool`/`DateTime` filter value compares against the event's canonical text form, without ClickHouse's cross-representation coercion (`true`, not `1`); ordering predicates (`_gt`/`_lt`) are schema-informed — numeric columns compare numerically (matching ClickHouse), other types lexicographically — which is best-effort for exotic types stored as numbers (a documented boundary in access-control.mdx), with every ambiguous or uncomparable value failing closed (the row is withheld). Replay (gap-fill) applies the same per-connection row check. The resource limits (`max_rows`, `max_execution_time`, …) remain a query-path property and are still **not** applied to the stream (a separate, documented boundary). Supersedes the "stream path applies no row-level filter" invariant noted in the #294/#353 Changed entry below. - **Policy `_in` is now enforced on both the row-`filter` and insert-`check` paths, closing a fail-open row-security gap** (`internal/policy/policy.go`, `internal/api/ingest.go`, `docs/src/content/docs/access-control.mdx`, plus tests in `internal/policy/policy_test.go`, `internal/api/ingest_test.go`): closes #224. The `Filter` schema accepted `_in` but the engine never read it: on the row-`filter`/SELECT path `resolveFilters` had no `_in` branch, so a row-security filter like `tenant_id: { _in: … }` produced **no `WHERE` predicate** and the role saw every row instead of its tenant subset (a fail-open, same family as #223); on the `check`/INSERT path only `_eq` was honored, silently dropping any other operator. `_in` now takes a single claim that resolves to a JSON **array** (the multi-tenant case — a token's `tenant_ids` list) and emits `col IN (?, …)` with one bound param per element; a scalar claim is a one-element set, and an empty/absent claim matches **no rows** (fail-closed) rather than widening to all of them. On the insert path an `_in` check requires the column be present and one of the set — there is no single value to auto-inject as `_eq` does, so an omitted column is rejected (`403 check failed`). The comparison operators are enforced on `filter` (`_eq`/`_neq`/`_gt`/`_lt`/`_in` all produce predicates now, so nothing is rejected there) and, on `check`, `_neq`/`_gt`/`_lt` become a loud config-load rejection (no insert-time semantics; `check` honors `_eq` + `_in`). The `_in` value stays a single templated string in the wire schema (Go `Filter.In`, SDK `PolicyFilter._in`), matching the established "set = array" shape of the caller-query `in` operator. - **`denied_aggregations` is now enforced case-insensitively against the caller-supplied function name, closing a policy bypass** (`internal/policy/policy.go`, plus tests in `internal/policy/policy_test.go`): closes #318. `IsAggregationAllowed` lower-cased the aggregation name only *after* the deny-list loop and the empty-allow-list early return, so the deny check compared a lower-cased deny entry against the raw caller input — a denied aggregation slipped past simply by changing case (`SUM` bypassed a `sum` deny entry, and with an empty allow list the call was then permitted). `isValidAggFn` already accepts any casing and the SQL builder emits the function name verbatim, so the denied aggregation actually executed. The case fold now happens once, before the deny check, so deny wins regardless of caller casing — matching the case-insensitive contract the access-control docs already specified. - **A role's structured-query resource caps are now enforced server-side by ClickHouse, so a public read can't outrun its budget during a server-side scan/merge/aggregation phase** (`internal/api/ch_settings.go` (new), `internal/api/structured_query.go`, `internal/policy/policy.go`, `internal/policy/scalars.go` (new), `internal/config/config.go`, `internal/query/builder.go`, `cmd/wavehouse/main.go`, `config.yaml`, `clients/ts/src/types.ts`, `docs/src/content/docs/{access-control.mdx,configuration.mdx}`, plus tests in `internal/api/ch_settings_test.go`, `internal/policy/{policy,scalars}_test.go`, `internal/config/config_test.go`, `internal/query/builder_test.go`, `tests/integration/query_limits_test.go` (all new/expanded), `tests/e2e/sdk/query.test.ts`): closes #316. The native ClickHouse connection passed **no per-query `Settings`**, so a read's policy caps bound it only client-side — a Go `context` deadline (which cancels only while the client is reading result blocks) plus a SQL `LIMIT`. Memory and rows scanned were **never bounded server-side**, so a heavy aggregation could allocate gigabytes of state or scan an entire table well within the time budget; and because `clickhouse-go` derives a `max_execution_time` setting from the context deadline only for deadlines `> 1s`, a sub-second time cap reached ClickHouse with no server-side time bound at all. The structured-query path now attaches per-query `Settings` derived from the role's resolved permissions: `max_execution_time` (fractional seconds, emitted explicitly so the sub-second case is enforced), `max_result_rows` + `result_overflow_mode=throw` (defense-in-depth behind the SQL `LIMIT`), `max_rows_to_read` + `read_overflow_mode=throw`, and `max_memory_usage` — so a query that exceeds its budget is rejected by the server (ClickHouse codes 158 / 241) rather than running to completion. **Boundary:** WaveHouse owns the *dynamic, per-role* caps (sent as per-query settings); the *global, static* backstop — which applies to every query including named pipes and raw admin SQL — is configured in **ClickHouse's own settings profiles and quotas** (documented in `configuration.mdx`), composes with the per-role caps, and holds even against a WaveHouse bug. **Schema (pre-launch):** the per-role policy fields are human-readable in / numeric out — `max_execution_time_ms` (int) → **`max_execution_time`** (set as a duration string `"5s"` or a bare ms number; read back as ms), the new **`max_memory_usage`** (set as a size string `"4GiB"` — IEC/SI respected via `dustin/go-humanize`, so `4GB` ≠ `4GiB` — or a bare byte number; read back as bytes), and the new **`max_rows_to_read`** (int), backed by two small `Millis`/`ByteSize` types in the `policy` package. The formerly hard-coded `query.DefaultMaxRows = 10000` result-LIMIT becomes the documented, tunable `query.default_max_rows` config knob (`Build` takes it as a parameter). Raw admin SQL remains unbounded by WaveHouse (governed by ClickHouse). Verified RED before the fix: the handler-level integration test confirmed a capped read returned the full result set when the settings weren't sent. diff --git a/docs/src/content/docs/access-control.mdx b/docs/src/content/docs/access-control.mdx index faae1027..3804b0af 100644 --- a/docs/src/content/docs/access-control.mdx +++ b/docs/src/content/docs/access-control.mdx @@ -375,7 +375,7 @@ The same policy drives every data path, but not every field is meaningful on eve | Named pipe | `GET/POST /v1/pipes/{name}` | per-pipe `allowed_roles` (not the policy engine; see [Named Pipes](/pipes)). Resource limits come from ClickHouse's [server-wide settings](/configuration#server-side-resource-limits), not per-role policy caps | :::caution[Live streams enforce column and row policy, but not resource limits] -SSE subscribers are checked for table-level `select` permission, have denied columns stripped from each event, and — like the query path — receive only the rows their role's row-level `filter` predicates admit, evaluated per subscriber against their JWT claims. Equality and set predicates (`_eq`, `_neq`, `_in`) — the usual tenant/user scoping — are enforced exactly. Ordering predicates (`_gt`, `_lt`) are best-effort: the stream compares numeric columns numerically (matching ClickHouse) but lacks ClickHouse's full per-type coercion, so an ordering filter on an exotic type (a `Decimal`/`Int128` beyond `float64` precision, or a `DateTime` ingested as a Unix number) can admit or withhold a row the query path wouldn't. The resource limits (`max_rows`, `max_execution_time`, `max_rows_to_read`, `max_memory_usage`) remain a property of the SQL query path and are **not** applied to the live event stream; if those caps are part of a role's isolation story, don't rely on them over the stream. +SSE subscribers are checked for table-level `select` permission, have denied columns stripped from each event, and — like the query path — receive only the rows their role's row-level `filter` predicates admit, evaluated per subscriber against their JWT claims. Equality and set predicates (`_eq`, `_neq`, `_in`) — the usual tenant/user scoping — are enforced exactly: numeric ties resolve at full precision (an ID beyond `float64`'s 2^53 never falsely matches a neighbor) and an unparseable or `NaN` operand withholds the row. One representation caveat: a `Bool` (or `DateTime`) value compares by its canonical text form, without ClickHouse's cross-representation coercion — write the filter value the way your events carry it (`true`, not `1`). Ordering predicates (`_gt`, `_lt`) are best-effort: the stream compares numeric columns numerically (matching ClickHouse) but lacks ClickHouse's full per-type coercion, so an ordering filter on an exotic type (a `Decimal`/`Int128` beyond `float64` precision, or a `DateTime` ingested as a Unix number) can admit or withhold a row the query path wouldn't. The resource limits (`max_rows`, `max_execution_time`, `max_rows_to_read`, `max_memory_usage`) remain a property of the SQL query path and are **not** applied to the live event stream; if those caps are part of a role's isolation story, don't rely on them over the stream. ::: ## Managing the policy diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index f35d5e80..c13b49de 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -85,7 +85,7 @@ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with Request The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https://github.com/Wave-RF/WaveHouse/issues/294)) lives next to the keepalive primitives it shares. One abstraction per file. -- **hub.go** — `Hub`, the event fan-out. Subscribers register under `(topic, role)`; `Broadcast` decodes each event once, applies each subscribed role's column policy once, builds one SSE frame per role, and fans it to every member of that role's `Bucket` — collapsing the prior per-subscriber `unmarshal → evaluate → filter → marshal` into one pass per distinct `(role, table)` output shape (the [#294](https://github.com/Wave-RF/WaveHouse/issues/294) lever; the measured ceiling was ~2 270 deliveries/s from re-projecting per subscriber). The column projection is claims-independent, so it is shared across a role's whole bucket; the role's row-level `filter` predicate is not — it is resolved against each subscriber's JWT claims, so for a role that carries a filter `Broadcast` keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (`ResolvedPermissions.RowVisible`, evaluated against the full event via the numeric-aware comparison seeded from the schema registry). This is the [#319](https://github.com/Wave-RF/WaveHouse/issues/319) fix that closes the query/stream row-level-security drift; roles without a filter keep the pure once-per-role fast path. `ReplayFrame` shares the same projection and per-connection row check for the handler's gap-fill. +- **hub.go** — `Hub`, the event fan-out. Subscribers register under `(topic, role)`; `Broadcast` decodes each event once, applies each subscribed role's column policy once, builds one SSE frame per role, and fans it to every member of that role's `Bucket` — collapsing the prior per-subscriber `unmarshal → evaluate → filter → marshal` into one pass per distinct `(role, table)` output shape (the [#294](https://github.com/Wave-RF/WaveHouse/issues/294) lever; the measured ceiling was ~2 270 deliveries/s from re-projecting per subscriber). The column projection is claims-independent, so it is shared across a role's whole bucket; the role's row-level `filter` predicate is not — it is resolved against each subscriber's JWT claims, so for a role that carries a filter `Broadcast` keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (`CompiledRowFilter.RowVisible`, evaluated against the full event via the numeric-aware comparison seeded from the schema registry). This is the [#319](https://github.com/Wave-RF/WaveHouse/issues/319) fix that closes the query/stream row-level-security drift; roles without a filter keep the pure once-per-role fast path. `ReplayFrame` shares the same projection and per-connection row check for the handler's gap-fill. - **subscriber.go** — `Subscriber`, the per-connection handle. It owns a single ready-to-write outbound queue of `Frame`s (each tagged with its `kind`, so the handler labels the write where it happens): producers — the keepalive wheel and the event `Hub` — fan frames in with `Send` (non-blocking; a full queue drops and the `Hub` counts it), and the handler drains `Frames()` to the client verbatim. The queue is sized for buffering live events (cap 64, up from the keepalive-only cap 1; #152 will make it a knob), and an `Evicted()` channel is the seam the slow-consumer follow-up closes to disconnect a wedged consumer. - **bucket.go** — `Bucket`, the reusable fan-out primitive: a concurrency-safe set of subscribers. `Push` delivers a shared `Frame` to each fire-and-forget (the keepalive wheel's ring); `Snapshot` exposes the members so the event `Hub` can fan out while inspecting each `Send` result (to count drops). The `Hub` holds one `Bucket` per `(topic, role)` so a projected frame is built once and sent to every member instead of re-projected per subscriber. - **heartbeat.go** — The keepalive wheel (`Heartbeater`). A single process-wide ticker fans a minimal `:` comment across the ring of `Bucket`s, waking ~1/N of live streams per tick so the writes don't synchronize. The effective per-connection keepalive period is `stream.keepalive_interval` (the wheel ticks every `keepalive_interval ÷ keepalive_buckets`, so one rotation spans the interval); the owning handler goroutine does the actual write, so the shared ticker never touches a `ResponseWriter` directly. diff --git a/internal/policy/rowfilter.go b/internal/policy/rowfilter.go index 12983ffe..92101851 100644 --- a/internal/policy/rowfilter.go +++ b/internal/policy/rowfilter.go @@ -1,6 +1,8 @@ package policy import ( + "math" + "math/big" "strconv" "strings" ) @@ -150,7 +152,9 @@ func compareScalar(rowVal any, filterVal string, numeric bool) (int, bool) { if numeric { a, err1 := strconv.ParseFloat(s, 64) b, err2 := strconv.ParseFloat(filterVal, 64) - if err1 != nil || err2 != nil { + // NaN must be rejected explicitly: ParseFloat accepts "NaN", and NaN's + // three-way comparison reads as "equal to everything" below — a fail-open. + if err1 != nil || err2 != nil || math.IsNaN(a) || math.IsNaN(b) { return 0, false } switch { @@ -159,12 +163,31 @@ func compareScalar(rowVal any, filterVal string, numeric bool) (int, bool) { case a > b: return 1, true default: - return 0, true + // Float equality is not proof of equality: distinct integers beyond + // 2^53 (string-encoded on ingest exactly to survive JS precision loss) + // collapse to one float64, and rounding is monotonic so only the + // equal case is in doubt. Resolve the tie at full precision. + return compareExact(s, filterVal) } } return strings.Compare(s, filterVal), true } +// compareExact compares two numeric strings at arbitrary precision — the tie-break +// for operands float64 cannot tell apart. ok=false when either side isn't an exact +// rational (±Inf, malformed), failing the predicate closed. +func compareExact(a, b string) (int, bool) { + ra, ok := new(big.Rat).SetString(a) + if !ok { + return 0, false + } + rb, ok := new(big.Rat).SetString(b) + if !ok { + return 0, false + } + return ra.Cmp(rb), true +} + // scalarString renders a JSON-decoded scalar as the canonical string compared // against a (string-valued) filter. Non-scalars (arrays, objects, null) return // ok=false so the predicate fails closed rather than guessing. JSON numbers arrive diff --git a/internal/policy/rowfilter_test.go b/internal/policy/rowfilter_test.go index 10707448..8c7a804f 100644 --- a/internal/policy/rowfilter_test.go +++ b/internal/policy/rowfilter_test.go @@ -78,6 +78,40 @@ func TestRowVisible_NumericEquality_FloatFormatting(t *testing.T) { assert.False(t, perms.RowVisible(map[string]any{"amount": float64(101)}, num)) } +// TestRowVisible_NaN_FailsClosed: strconv.ParseFloat accepts "NaN", and NaN's +// three-way comparison would otherwise read as "equal to everything" — a fail-open +// that delivers a row the query path's WHERE excludes. Either operand parsing to +// NaN must withhold the row instead. +func TestRowVisible_NaN_FailsClosed(t *testing.T) { + t.Parallel() + num := map[string]bool{"amount": true} + + byRow := evalRowFilter(t, map[string]Filter{"amount": {Eq: new("100")}}, nil) + assert.False(t, byRow.RowVisible(map[string]any{"amount": "NaN"}, num), "NaN row value must not equal 100") + + byClaim := evalRowFilter(t, map[string]Filter{"amount": {Eq: new("{{ jwt.cap }}")}}, map[string]any{"cap": "NaN"}) + assert.False(t, byClaim.RowVisible(map[string]any{"amount": float64(100)}, num), "NaN claim must not match any row") + + neq := evalRowFilter(t, map[string]Filter{"amount": {Neq: new("NaN")}}, nil) + assert.False(t, neq.RowVisible(map[string]any{"amount": float64(100)}, num), "NaN is uncomparable, so even != fails closed") +} + +// TestRowVisible_NumericEquality_ExactBeyondFloat64: ingest accepts string-encoded +// numerics precisely so 64-bit IDs survive JS precision loss; equality must not +// collapse distinct IDs that round to the same float64 (adjacent values past 2^53). +func TestRowVisible_NumericEquality_ExactBeyondFloat64(t *testing.T) { + t.Parallel() + num := map[string]bool{"id": true} + + perms := evalRowFilter(t, map[string]Filter{"id": {Eq: new("9007199254740993")}}, nil) + assert.False(t, perms.RowVisible(map[string]any{"id": "9007199254740992"}, num), "float64-equal neighbors are not equal") + assert.True(t, perms.RowVisible(map[string]any{"id": "9007199254740993"}, num)) + assert.True(t, perms.RowVisible(map[string]any{"id": "9007199254740993.0"}, num), "same value in a different rendering still matches") + + neq := evalRowFilter(t, map[string]Filter{"id": {Neq: new("9007199254740993")}}, nil) + assert.True(t, neq.RowVisible(map[string]any{"id": "9007199254740992"}, num), "the exact tie-break keeps distinct IDs unequal for !=") +} + func TestRowVisible_NilReceiver_AllVisible(t *testing.T) { t.Parallel() var perms *ResolvedPermissions diff --git a/internal/stream/hub.go b/internal/stream/hub.go index eae6da8c..c8af8390 100644 --- a/internal/stream/hub.go +++ b/internal/stream/hub.go @@ -19,7 +19,7 @@ import ( // resolved against each subscriber's claims, so two subscribers of the same role can // be entitled to different rows. For a role that carries a row-filter, Broadcast // therefore keeps the shared column projection but evaluates row visibility PER -// subscriber (ResolvedPermissions.RowVisible) before delivering — closing the +// subscriber (CompiledRowFilter.RowVisible) before delivering — closing the // query/stream RLS drift in #319. Roles without a row-filter keep the pure // once-per-role fast path unchanged. See projectColumns. type Hub struct { @@ -241,8 +241,9 @@ func (h *Hub) ReplayFrame(role string, claims map[string]any, raw []byte) (Frame // role+table policy entry (AllowColumns/DenyColumns), never from claims, so this // frame is byte-identical for every subscriber of a (role, table) — the shared // projection that lets one serialization serve a whole bucket. Row-level security is -// NOT applied here; the caller checks perms.RowVisible per subscriber (with that -// subscriber's claims) when perms.HasRowFilter(). ok=false means skip: the role can't +// NOT applied here; when perms.HasRowFilter(), the caller compiles the role's filter +// once (policy.CompileRowFilter) and checks its RowVisible per subscriber with that +// subscriber's claims. ok=false means skip: the role can't // read the table, or the payload is unusable. perms is nil for the legacy no-policy // passthrough (which has no row-filter). func projectColumns(p *policy.Policy, filter bool, role string, evt *ingest.EventMessage, raw []byte, decoded bool) (wire []byte, perms *policy.ResolvedPermissions, ok bool) { From 2379bc48653399ea986703392612c9f612cc9bfd Mon Sep 17 00:00:00 2001 From: taitelee Date: Thu, 9 Jul 2026 08:39:52 -0400 Subject: [PATCH 9/9] refactor(policy): drop compiled row filter; evaluate per subscriber --- CHANGELOG.md | 2 +- docs/src/content/docs/architecture.md | 2 +- internal/policy/policy.go | 302 +++++--------------------- internal/policy/rowfilter.go | 68 +----- internal/policy/rowfilter_test.go | 125 ----------- internal/stream/hub.go | 18 +- internal/stream/hub_test.go | 45 +--- tests/e2e/sdk/streaming.test.ts | 2 +- 8 files changed, 75 insertions(+), 489 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a41fc54d..cbcf5c81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security -- **Live SSE streams now apply a role's row-`filter` per subscriber, closing a query/stream row-level-security drift** (`internal/stream/hub.go`, `internal/stream/subscriber.go`, `internal/policy/policy.go`, `internal/policy/rowfilter.go` (new), `internal/discovery/validation.go`, `internal/api/stream.go`, `cmd/wavehouse/main.go`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/architecture.md`, plus tests in `internal/policy/rowfilter_test.go` (new), `internal/discovery/validation_test.go`, `internal/stream/hub_test.go`): closes #319. The SSE delivery path stripped denied columns but never applied a role's row-level `filter` predicate, so a subscriber received rows the structured-query path would have filtered out for that same role — a data-exposure on the streaming surface for any table that combines a row-policy with a shared or role-scoped stream (harmless on the public Stats table today, which carries no restrictive row-policy, but real for any private/PII table fronted by a stream). The row-filter is now resolved once into predicates that feed **both** read surfaces — the query path renders them to SQL, the stream evaluates them in memory (`policy.CompileRowFilter` → `CompiledRowFilter.RowVisible`, compiled once per role bucket and claim-bound per subscriber) — so the two can't drift (the row-level analogue of the shared `IsColumnAllowed` decision from #223). Because a row-filter resolves against each subscriber's JWT claims, the #294/#353 once-per-role projection is now claims-aware: a role **without** a filter keeps the pure once-per-role fast path unchanged (zero regression on the public stream), while a role **with** a filter keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (evaluated against the full event, so a filter may key on a column the role can't select). Equality and set predicates (`_eq`/`_neq`/`_in` — the tenant/user scoping row-security actually uses) are enforced exactly — numeric ties beyond `float64` precision resolve at arbitrary precision (`math/big`), so the string-encoded 64-bit IDs ingest accepts to survive JS precision loss never falsely collide, and a `NaN` operand fails closed rather than comparing equal to everything — with one representation caveat: a `Bool`/`DateTime` filter value compares against the event's canonical text form, without ClickHouse's cross-representation coercion (`true`, not `1`); ordering predicates (`_gt`/`_lt`) are schema-informed — numeric columns compare numerically (matching ClickHouse), other types lexicographically — which is best-effort for exotic types stored as numbers (a documented boundary in access-control.mdx), with every ambiguous or uncomparable value failing closed (the row is withheld). Replay (gap-fill) applies the same per-connection row check. The resource limits (`max_rows`, `max_execution_time`, …) remain a query-path property and are still **not** applied to the stream (a separate, documented boundary). Supersedes the "stream path applies no row-level filter" invariant noted in the #294/#353 Changed entry below. +- **Live SSE streams now apply a role's row-`filter` per subscriber, closing a query/stream row-level-security drift** (`internal/stream/hub.go`, `internal/stream/subscriber.go`, `internal/policy/policy.go`, `internal/policy/rowfilter.go` (new), `internal/discovery/validation.go`, `internal/api/stream.go`, `cmd/wavehouse/main.go`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/architecture.md`, plus tests in `internal/policy/rowfilter_test.go` (new), `internal/discovery/validation_test.go`, `internal/stream/hub_test.go`): closes #319. The SSE delivery path stripped denied columns but never applied a role's row-level `filter` predicate, so a subscriber received rows the structured-query path would have filtered out for that same role — a data-exposure on the streaming surface for any table that combines a row-policy with a shared or role-scoped stream (harmless on the public Stats table today, which carries no restrictive row-policy, but real for any private/PII table fronted by a stream). The row-filter is now resolved once into predicates that feed **both** read surfaces — the query path renders them to SQL, the stream evaluates them in memory (`ResolvedPermissions.RowVisible`, evaluated per subscriber against that subscriber's claims via the same `Evaluate` call the query path uses) — so the two can't drift (the row-level analogue of the shared `IsColumnAllowed` decision from #223). Because a row-filter resolves against each subscriber's JWT claims, the #294/#353 once-per-role projection is now claims-aware: a role **without** a filter keeps the pure once-per-role fast path unchanged (zero regression on the public stream), while a role **with** a filter keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (evaluated against the full event, so a filter may key on a column the role can't select). Equality and set predicates (`_eq`/`_neq`/`_in` — the tenant/user scoping row-security actually uses) are enforced exactly — numeric ties beyond `float64` precision resolve at arbitrary precision (`math/big`), so the string-encoded 64-bit IDs ingest accepts to survive JS precision loss never falsely collide, and a `NaN` operand fails closed rather than comparing equal to everything — with one representation caveat: a `Bool`/`DateTime` filter value compares against the event's canonical text form, without ClickHouse's cross-representation coercion (`true`, not `1`); ordering predicates (`_gt`/`_lt`) are schema-informed — numeric columns compare numerically (matching ClickHouse), other types lexicographically — which is best-effort for exotic types stored as numbers (a documented boundary in access-control.mdx), with every ambiguous or uncomparable value failing closed (the row is withheld). Replay (gap-fill) applies the same per-connection row check. The resource limits (`max_rows`, `max_execution_time`, …) remain a query-path property and are still **not** applied to the stream (a separate, documented boundary). Supersedes the "stream path applies no row-level filter" invariant noted in the #294/#353 Changed entry below. - **Policy `_in` is now enforced on both the row-`filter` and insert-`check` paths, closing a fail-open row-security gap** (`internal/policy/policy.go`, `internal/api/ingest.go`, `docs/src/content/docs/access-control.mdx`, plus tests in `internal/policy/policy_test.go`, `internal/api/ingest_test.go`): closes #224. The `Filter` schema accepted `_in` but the engine never read it: on the row-`filter`/SELECT path `resolveFilters` had no `_in` branch, so a row-security filter like `tenant_id: { _in: … }` produced **no `WHERE` predicate** and the role saw every row instead of its tenant subset (a fail-open, same family as #223); on the `check`/INSERT path only `_eq` was honored, silently dropping any other operator. `_in` now takes a single claim that resolves to a JSON **array** (the multi-tenant case — a token's `tenant_ids` list) and emits `col IN (?, …)` with one bound param per element; a scalar claim is a one-element set, and an empty/absent claim matches **no rows** (fail-closed) rather than widening to all of them. On the insert path an `_in` check requires the column be present and one of the set — there is no single value to auto-inject as `_eq` does, so an omitted column is rejected (`403 check failed`). The comparison operators are enforced on `filter` (`_eq`/`_neq`/`_gt`/`_lt`/`_in` all produce predicates now, so nothing is rejected there) and, on `check`, `_neq`/`_gt`/`_lt` become a loud config-load rejection (no insert-time semantics; `check` honors `_eq` + `_in`). The `_in` value stays a single templated string in the wire schema (Go `Filter.In`, SDK `PolicyFilter._in`), matching the established "set = array" shape of the caller-query `in` operator. - **`denied_aggregations` is now enforced case-insensitively against the caller-supplied function name, closing a policy bypass** (`internal/policy/policy.go`, plus tests in `internal/policy/policy_test.go`): closes #318. `IsAggregationAllowed` lower-cased the aggregation name only *after* the deny-list loop and the empty-allow-list early return, so the deny check compared a lower-cased deny entry against the raw caller input — a denied aggregation slipped past simply by changing case (`SUM` bypassed a `sum` deny entry, and with an empty allow list the call was then permitted). `isValidAggFn` already accepts any casing and the SQL builder emits the function name verbatim, so the denied aggregation actually executed. The case fold now happens once, before the deny check, so deny wins regardless of caller casing — matching the case-insensitive contract the access-control docs already specified. - **A role's structured-query resource caps are now enforced server-side by ClickHouse, so a public read can't outrun its budget during a server-side scan/merge/aggregation phase** (`internal/api/ch_settings.go` (new), `internal/api/structured_query.go`, `internal/policy/policy.go`, `internal/policy/scalars.go` (new), `internal/config/config.go`, `internal/query/builder.go`, `cmd/wavehouse/main.go`, `config.yaml`, `clients/ts/src/types.ts`, `docs/src/content/docs/{access-control.mdx,configuration.mdx}`, plus tests in `internal/api/ch_settings_test.go`, `internal/policy/{policy,scalars}_test.go`, `internal/config/config_test.go`, `internal/query/builder_test.go`, `tests/integration/query_limits_test.go` (all new/expanded), `tests/e2e/sdk/query.test.ts`): closes #316. The native ClickHouse connection passed **no per-query `Settings`**, so a read's policy caps bound it only client-side — a Go `context` deadline (which cancels only while the client is reading result blocks) plus a SQL `LIMIT`. Memory and rows scanned were **never bounded server-side**, so a heavy aggregation could allocate gigabytes of state or scan an entire table well within the time budget; and because `clickhouse-go` derives a `max_execution_time` setting from the context deadline only for deadlines `> 1s`, a sub-second time cap reached ClickHouse with no server-side time bound at all. The structured-query path now attaches per-query `Settings` derived from the role's resolved permissions: `max_execution_time` (fractional seconds, emitted explicitly so the sub-second case is enforced), `max_result_rows` + `result_overflow_mode=throw` (defense-in-depth behind the SQL `LIMIT`), `max_rows_to_read` + `read_overflow_mode=throw`, and `max_memory_usage` — so a query that exceeds its budget is rejected by the server (ClickHouse codes 158 / 241) rather than running to completion. **Boundary:** WaveHouse owns the *dynamic, per-role* caps (sent as per-query settings); the *global, static* backstop — which applies to every query including named pipes and raw admin SQL — is configured in **ClickHouse's own settings profiles and quotas** (documented in `configuration.mdx`), composes with the per-role caps, and holds even against a WaveHouse bug. **Schema (pre-launch):** the per-role policy fields are human-readable in / numeric out — `max_execution_time_ms` (int) → **`max_execution_time`** (set as a duration string `"5s"` or a bare ms number; read back as ms), the new **`max_memory_usage`** (set as a size string `"4GiB"` — IEC/SI respected via `dustin/go-humanize`, so `4GB` ≠ `4GiB` — or a bare byte number; read back as bytes), and the new **`max_rows_to_read`** (int), backed by two small `Millis`/`ByteSize` types in the `policy` package. The formerly hard-coded `query.DefaultMaxRows = 10000` result-LIMIT becomes the documented, tunable `query.default_max_rows` config knob (`Build` takes it as a parameter). Raw admin SQL remains unbounded by WaveHouse (governed by ClickHouse). Verified RED before the fix: the handler-level integration test confirmed a capped read returned the full result set when the settings weren't sent. diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index c13b49de..f35d5e80 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -85,7 +85,7 @@ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with Request The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https://github.com/Wave-RF/WaveHouse/issues/294)) lives next to the keepalive primitives it shares. One abstraction per file. -- **hub.go** — `Hub`, the event fan-out. Subscribers register under `(topic, role)`; `Broadcast` decodes each event once, applies each subscribed role's column policy once, builds one SSE frame per role, and fans it to every member of that role's `Bucket` — collapsing the prior per-subscriber `unmarshal → evaluate → filter → marshal` into one pass per distinct `(role, table)` output shape (the [#294](https://github.com/Wave-RF/WaveHouse/issues/294) lever; the measured ceiling was ~2 270 deliveries/s from re-projecting per subscriber). The column projection is claims-independent, so it is shared across a role's whole bucket; the role's row-level `filter` predicate is not — it is resolved against each subscriber's JWT claims, so for a role that carries a filter `Broadcast` keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (`CompiledRowFilter.RowVisible`, evaluated against the full event via the numeric-aware comparison seeded from the schema registry). This is the [#319](https://github.com/Wave-RF/WaveHouse/issues/319) fix that closes the query/stream row-level-security drift; roles without a filter keep the pure once-per-role fast path. `ReplayFrame` shares the same projection and per-connection row check for the handler's gap-fill. +- **hub.go** — `Hub`, the event fan-out. Subscribers register under `(topic, role)`; `Broadcast` decodes each event once, applies each subscribed role's column policy once, builds one SSE frame per role, and fans it to every member of that role's `Bucket` — collapsing the prior per-subscriber `unmarshal → evaluate → filter → marshal` into one pass per distinct `(role, table)` output shape (the [#294](https://github.com/Wave-RF/WaveHouse/issues/294) lever; the measured ceiling was ~2 270 deliveries/s from re-projecting per subscriber). The column projection is claims-independent, so it is shared across a role's whole bucket; the role's row-level `filter` predicate is not — it is resolved against each subscriber's JWT claims, so for a role that carries a filter `Broadcast` keeps the shared column projection but delivers it only to the subscribers whose claims admit each row (`ResolvedPermissions.RowVisible`, evaluated against the full event via the numeric-aware comparison seeded from the schema registry). This is the [#319](https://github.com/Wave-RF/WaveHouse/issues/319) fix that closes the query/stream row-level-security drift; roles without a filter keep the pure once-per-role fast path. `ReplayFrame` shares the same projection and per-connection row check for the handler's gap-fill. - **subscriber.go** — `Subscriber`, the per-connection handle. It owns a single ready-to-write outbound queue of `Frame`s (each tagged with its `kind`, so the handler labels the write where it happens): producers — the keepalive wheel and the event `Hub` — fan frames in with `Send` (non-blocking; a full queue drops and the `Hub` counts it), and the handler drains `Frames()` to the client verbatim. The queue is sized for buffering live events (cap 64, up from the keepalive-only cap 1; #152 will make it a knob), and an `Evicted()` channel is the seam the slow-consumer follow-up closes to disconnect a wedged consumer. - **bucket.go** — `Bucket`, the reusable fan-out primitive: a concurrency-safe set of subscribers. `Push` delivers a shared `Frame` to each fire-and-forget (the keepalive wheel's ring); `Snapshot` exposes the members so the event `Hub` can fan out while inspecting each `Send` result (to count drops). The `Hub` holds one `Bucket` per `(topic, role)` so a projected frame is built once and sent to every member instead of re-projected per subscriber. - **heartbeat.go** — The keepalive wheel (`Heartbeater`). A single process-wide ticker fans a minimal `:` comment across the ring of `Bucket`s, waking ~1/N of live streams per tick so the writes don't synchronize. The effective per-connection keepalive period is `stream.keepalive_interval` (the wheel ticks every `keepalive_interval ÷ keepalive_buckets`, so one rotation spans the interval); the owning handler goroutine does the actual write, so the shared ticker never touches a `ResponseWriter` directly. diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 2274198a..c0f5a4f3 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -108,14 +108,8 @@ func ResolveRole(p *Policy, role string) string { return p.DefaultRole } -// resolveRolePerms navigates the policy to the RolePermissions governing -// (role, table, operation), applying role resolution, the admin bypass, and -// default-deny. It returns admin=true for the unconditional-bypass role (perms is the -// zero value — the caller grants full access) and ok=false for any denial (perms is -// the zero value). Evaluate (query path) and CompileRowFilter (stream path) share -// this one navigation so they can never disagree on who is authorized or which policy -// entry applies. -func resolveRolePerms(p *Policy, role, table, operation string) (perms RolePermissions, admin, ok bool) { +// Evaluate resolves a policy for a given role, table, and operation against JWT claims. +func Evaluate(p *Policy, role, table, operation string, claims map[string]any) *ResolvedPermissions { // Map an empty/absent role to the configured default_role (no-op if none, // or if the policy is nil). A non-empty role is unchanged — roles never // inherit the default's permissions. @@ -131,20 +125,20 @@ func resolveRolePerms(p *Policy, role, table, operation string) (perms RolePermi // over HTTP), so a nil policy falls through to the deny just below. Mirrors // RoleAllowed's admin short-circuit on the pipe path. if IsAdmin(p, role) { - return RolePermissions{}, true, false + return &ResolvedPermissions{Allowed: true} } // Beyond here the role is non-admin (non-empty only if it carried a concrete // role or matched a real default_role); any failure to find a matching entry // is a plain deny. if p == nil { - return RolePermissions{}, false, false + return &ResolvedPermissions{Allowed: false} } - tp, found := p.Tables[table] - if !found { + tp, ok := p.Tables[table] + if !ok { // No policy for this table — default deny. - return RolePermissions{}, false, false + return &ResolvedPermissions{Allowed: false} } var rolePerms map[string]RolePermissions @@ -154,11 +148,11 @@ func resolveRolePerms(p *Policy, role, table, operation string) (perms RolePermi case "insert": rolePerms = tp.Insert default: - return RolePermissions{}, false, false + return &ResolvedPermissions{Allowed: false} } if rolePerms == nil { - return RolePermissions{}, false, false + return &ResolvedPermissions{Allowed: false} } // An empty/absent role must never match a role entry — a roleless request is @@ -167,18 +161,9 @@ func resolveRolePerms(p *Policy, role, table, operation string) (perms RolePermi // role key in a policy therefore grants nothing: the policy-side twin of the // empty-AllowedRoles-entry footgun closed for pipes in #159. Matching is // exact — there is no "*" any-role wildcard. - if role == "" { - return RolePermissions{}, false, false - } - perms, ok = rolePerms[role] - return perms, false, ok -} - -// Evaluate resolves a policy for a given role, table, and operation against JWT claims. -func Evaluate(p *Policy, role, table, operation string, claims map[string]any) *ResolvedPermissions { - perms, admin, ok := resolveRolePerms(p, role, table, operation) - if admin { - return &ResolvedPermissions{Allowed: true} + perms, ok := RolePermissions{}, false + if role != "" { + perms, ok = rolePerms[role] } if !ok { return &ResolvedPermissions{Allowed: false} @@ -196,16 +181,24 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * MaxMemoryUsage: perms.MaxMemoryUsage, } - // Resolve the row-filter once into predicates, then render both read surfaces - // from that single source so they can't drift: the query path binds them into a - // SQL WHERE here; the stream path evaluates the same compiled predicates in memory - // (CompiledRowFilter.RowVisible). resolveFilterPredicates fails closed on a - // bind-unsafe column (see there). + // Resolve filters into WHERE clause. A bind-unsafe filter column can't be + // emitted safely — a '?' in it would shift clickhouse-go's positional value + // binding, including this RLS filter's own bound value — so deny the role + // fail-closed rather than drop the predicate (which would widen row access) + // or emit a mis-bound query. validateRolePerms rejects such a policy at write + // time; this guards the query path as defense-in-depth (Evaluate does not + // re-validate the policy it is handed). if len(perms.Filter) > 0 { - preds, deny := resolveFilterPredicates(perms.Filter, claims) - if deny { - return &ResolvedPermissions{Allowed: false} + for col := range perms.Filter { + if chsql.BindUnsafe(col) { + return &ResolvedPermissions{Allowed: false} + } } + // Resolve the row-filter once into predicates, then render both read surfaces + // from that single source so they can't drift: the query path binds them into + // a SQL WHERE here; the stream path evaluates the same predicates in memory + // (ResolvedPermissions.RowVisible). + preds := resolvePredicates(perms.Filter, claims) resolved.rowFilter = preds clauses, params := predicatesToSQL(preds) if len(clauses) > 0 { @@ -232,234 +225,42 @@ func Evaluate(p *Policy, role, table, operation string, claims map[string]any) * return resolved } -// resolveFilterPredicates resolves a role's row-filter map into predicates for the -// query path, first failing closed (deny=true) if any filter column is bind-unsafe: a -// '?' in the column would shift clickhouse-go's positional value binding — including -// this filter's own bound value — so the role is denied rather than the predicate -// dropped (which would widen row access) or a mis-bound query emitted. validateRolePerms -// rejects such a policy at write time; this is defense-in-depth (the resolver does not -// re-validate the policy it is handed). The stream path applies the same bind-unsafe -// gate in CompileRowFilter, so both read surfaces reject the same unsafe policy. -func resolveFilterPredicates(filters map[string]Filter, claims map[string]any) (preds []resolvedPredicate, deny bool) { - if filterBindUnsafe(filters) { - return nil, true - } - return resolvePredicates(filters, claims), false -} - -// filterBindUnsafe reports whether any filter column can't be bound safely (see -// resolveFilterPredicates for why that denies the role). Claims-independent, so the -// stream can check it once per bucket in CompileRowFilter. -func filterBindUnsafe(filters map[string]Filter) bool { - for col := range filters { - if chsql.BindUnsafe(col) { - return true - } - } - return false -} - -// ----------------------------------------------------------------------------- -// Compiled row filter: claims-independent structure, per-subscriber claim binding. -// -// The stream fan-out resolves a role's row-filter ONCE per bucket into compiled -// predicates (CompileRowFilter), then binds each subscriber's claims at evaluation -// time (CompiledRowFilter.RowVisible). It is the deferred-binding twin of -// resolvePredicates, which eagerly resolves for the query path's single claim set. -// The two must agree on every (filter, claims) pair — that parity is pinned by -// TestCompiledRowFilter_MatchesEvaluatePath. The optimization is that a whole -// {{ jwt.path }} value binds via a map walk (navigateClaims) and a constant binds to -// itself, so a filtered high-fan-out topic pays no per-subscriber regexp/predicate -// allocation. -// ----------------------------------------------------------------------------- - -type valueKind uint8 - -const ( - valConstant valueKind = iota // literal, no claim template - valClaim // exactly one {{ jwt.path }} — bound via navigateClaims - valTemplate // text with embedded {{ jwt.… }} — bound via resolveTemplate -) - -// compiledValue is a filter value with its claim-template structure classified once, -// so binding a subscriber's claims is a map walk (valClaim) or a no-op (valConstant) -// rather than a regexp pass. -type compiledValue struct { - kind valueKind - s string // valConstant literal, or valTemplate raw text - path []string // valClaim: the pre-split jwt claim path -} - -// compileScalarValue classifies an _eq/_neq/_gt/_lt value the way resolveTemplate reads -// it — a substring replace, with a whole-value (no surrounding text) {{ jwt.path }} -// short-circuited to a direct claim lookup. It matches on the UNtrimmed value so a -// space-padded ref stays a template, exactly as resolveTemplate would keep the spaces. -func compileScalarValue(v string) compiledValue { - if m := wholeClaimRe.FindStringSubmatch(v); m != nil { - return compiledValue{kind: valClaim, path: strings.Split(m[1], ".")} - } - if claimTemplateRe.MatchString(v) { - return compiledValue{kind: valTemplate, s: v} - } - return compiledValue{kind: valConstant, s: v} -} - -// compileInValue classifies an _in value the way resolveInValues reads it: a -// TrimSpace'd whole {{ jwt.path }} is a claim list; anything else is a single -// (possibly templated) value. -func compileInValue(v string) compiledValue { - if m := wholeClaimRe.FindStringSubmatch(strings.TrimSpace(v)); m != nil { - return compiledValue{kind: valClaim, path: strings.Split(m[1], ".")} - } - if claimTemplateRe.MatchString(v) { - return compiledValue{kind: valTemplate, s: v} - } - return compiledValue{kind: valConstant, s: v} -} - -// bindScalar resolves a scalar value against claims — the deferred twin of -// resolveTemplate. A whole-claim ref returns the claim's string form (no allocation -// when the claim is already a string; fmt.Sprint otherwise, matching resolveTemplate), -// and a missing claim resolves to "" exactly as resolveTemplate does. -func (cv compiledValue) bindScalar(claims map[string]any) string { - switch cv.kind { - case valClaim: - val := navigateClaims(claims, cv.path) - if val == nil { - return "" - } - if s, ok := val.(string); ok { - return s - } - return fmt.Sprint(val) - case valTemplate: - return resolveTemplate(cv.s, claims) - case valConstant: - return cv.s - default: - return cv.s // unreachable: every valueKind has an explicit case above - } -} - -// bindIn resolves an _in value against claims into the set of bound values (the -// deferred twin of resolveInValues). A claim list yields one bound value per element; -// any other value yields the single scalar-bound value. -func (cv compiledValue) bindIn(claims map[string]any) []string { - if cv.kind == valClaim { - switch v := navigateClaims(claims, cv.path).(type) { - case nil: - return nil - case []any: - out := make([]string, 0, len(v)) - for _, e := range v { - out = append(out, fmt.Sprint(e)) - } - return out - default: - return []string{fmt.Sprint(v)} - } - } - return []string{cv.bindScalar(claims)} -} - -// compiledPredicate is one row-filter comparison with its value structure compiled but -// the claim binding deferred to evaluation time. -type compiledPredicate struct { +// resolvedPredicate is one row-filter comparison with its claim templates already +// resolved to concrete string values — the shared, render-agnostic form the query +// path turns into SQL (predicatesToSQL) and the stream path evaluates in memory +// (RowVisible). Op is one of "=", "!=", ">", "<", "in". Values holds one element +// for the scalar operators and zero-or-more for "in" (empty ⇒ matches no rows). +type resolvedPredicate struct { Column string Op string - Value compiledValue + Values []string } -// compilePredicates compiles a role's row-filter into per-subscriber-bindable -// predicates, mirroring resolvePredicates' column/operator expansion (and order) so the -// compiled and resolved paths produce the same predicate set. -func compilePredicates(filters map[string]Filter) []compiledPredicate { - var preds []compiledPredicate +// resolvePredicates resolves each filter's claim templates once into predicates. +// Both read surfaces derive from this single result so they can't drift; the +// operator order within a column (=, !=, >, <, in) mirrors the former inline SQL. +func resolvePredicates(filters map[string]Filter, claims map[string]any) []resolvedPredicate { + var preds []resolvedPredicate for col, f := range filters { if f.Eq != nil { - preds = append(preds, compiledPredicate{col, "=", compileScalarValue(*f.Eq)}) + preds = append(preds, resolvedPredicate{col, "=", []string{resolveTemplate(*f.Eq, claims)}}) } if f.Neq != nil { - preds = append(preds, compiledPredicate{col, "!=", compileScalarValue(*f.Neq)}) + preds = append(preds, resolvedPredicate{col, "!=", []string{resolveTemplate(*f.Neq, claims)}}) } if f.Gt != nil { - preds = append(preds, compiledPredicate{col, ">", compileScalarValue(*f.Gt)}) + preds = append(preds, resolvedPredicate{col, ">", []string{resolveTemplate(*f.Gt, claims)}}) } if f.Lt != nil { - preds = append(preds, compiledPredicate{col, "<", compileScalarValue(*f.Lt)}) + preds = append(preds, resolvedPredicate{col, "<", []string{resolveTemplate(*f.Lt, claims)}}) } if f.In != nil { - preds = append(preds, compiledPredicate{col, "in", compileInValue(*f.In)}) + preds = append(preds, resolvedPredicate{col, "in", toStrings(resolveInValues(*f.In, claims))}) } } return preds } -// CompiledRowFilter is a role/table's row-level-security filter compiled once -// (claims-independent) so the stream fan-out can reuse it across every subscriber in a -// bucket: resolve it per bucket with CompileRowFilter, then call RowVisible per -// subscriber with that subscriber's claims. It binds claims at evaluation time, so a -// filtered high-fan-out topic pays no per-subscriber predicate resolution — the -// deferred-binding counterpart to the query path's resolvePredicates (which binds a -// single claim set eagerly for SQL). -type CompiledRowFilter struct { - allowed bool - preds []compiledPredicate -} - -// CompileRowFilter compiles the row-filter governing (role, table, operation), -// claims-independently. It shares Evaluate's role resolution, admin bypass, and -// default-deny (via resolveRolePerms) and the same bind-unsafe gate, so it agrees with -// the query path on who is authorized and which policy is rejected. Compile once per -// role bucket; the per-subscriber cost is then only RowVisible. -func CompileRowFilter(p *Policy, role, table, operation string) *CompiledRowFilter { - perms, admin, ok := resolveRolePerms(p, role, table, operation) - if admin { - return &CompiledRowFilter{allowed: true} // admin: no predicates ⇒ every row visible - } - if !ok { - return &CompiledRowFilter{allowed: false} - } - if filterBindUnsafe(perms.Filter) { - return &CompiledRowFilter{allowed: false} // fail closed, as Evaluate does - } - return &CompiledRowFilter{allowed: true, preds: compilePredicates(perms.Filter)} -} - -// resolvedPredicate is one row-filter comparison with its claim templates already -// resolved to concrete string values — the shared, render-agnostic form the query -// path turns into SQL (predicatesToSQL) and the stream path evaluates in memory -// (RowVisible). Op is one of "=", "!=", ">", "<", "in". Values holds one element -// for the scalar operators and zero-or-more for "in" (empty ⇒ matches no rows). -type resolvedPredicate struct { - Column string - Op string - Values []string -} - -// resolvePredicates resolves each filter's claim templates into predicates for the -// query path. It compiles the filter — the single, claims-independent source the stream -// path also uses (CompileRowFilter) — then binds this request's claims, so the two read -// surfaces derive from one place and can't drift. Operator order within a column -// (=, !=, >, <, in) mirrors compilePredicates and the former inline SQL. -func resolvePredicates(filters map[string]Filter, claims map[string]any) []resolvedPredicate { - var preds []resolvedPredicate - for _, cp := range compilePredicates(filters) { - preds = append(preds, cp.resolve(claims)) - } - return preds -} - -// resolve binds a compiled predicate's claim value(s) into a resolvedPredicate — the -// eager, query-path counterpart to compiledPredicate.visible (which the stream -// evaluates against the row in memory instead of materializing this form). -func (cp compiledPredicate) resolve(claims map[string]any) resolvedPredicate { - if cp.Op == "in" { - return resolvedPredicate{cp.Column, "in", cp.Value.bindIn(claims)} - } - return resolvedPredicate{cp.Column, cp.Op, []string{cp.Value.bindScalar(claims)}} -} - // predicatesToSQL renders resolved predicates into WHERE clauses and bound params. func predicatesToSQL(preds []resolvedPredicate) ([]string, []any) { var clauses []string @@ -497,6 +298,19 @@ func resolveFilters(filters map[string]Filter, claims map[string]any) ([]string, return predicatesToSQL(resolvePredicates(filters, claims)) } +// toStrings normalizes resolveInValues' []any (already stringified elements) to the +// []string a resolvedPredicate carries. +func toStrings(vals []any) []string { + if len(vals) == 0 { + return nil + } + out := make([]string, len(vals)) + for i, v := range vals { + out[i] = fmt.Sprint(v) + } + return out +} + // resolveTemplate resolves {{ jwt.claim.path }} templates against JWT claims. // If a claim path cannot be resolved, the template placeholder is replaced with // an empty string to prevent "" from leaking into SQL filters. diff --git a/internal/policy/rowfilter.go b/internal/policy/rowfilter.go index 92101851..b008165b 100644 --- a/internal/policy/rowfilter.go +++ b/internal/policy/rowfilter.go @@ -37,6 +37,11 @@ func (p *ResolvedPermissions) RowVisible(row map[string]any, numericCols map[str if p == nil { return true } + // A denied role sees no rows — fail closed, mirroring the !Allowed guard on + // IsColumnAllowed, so a denied receiver never reads as "no filter ⇒ all visible". + if !p.Allowed { + return false + } for _, pred := range p.rowFilter { if !pred.matches(row, numericCols[pred.Column]) { return false @@ -77,69 +82,6 @@ func (pred resolvedPredicate) matches(row map[string]any, numeric bool) bool { } } -// HasRowFilter reports whether the compiled filter carries any predicate. A nil -// receiver, or an admin / unfiltered role, has none — the compiled twin of -// ResolvedPermissions.HasRowFilter. -func (c *CompiledRowFilter) HasRowFilter() bool { - return c != nil && len(c.preds) > 0 -} - -// RowVisible reports whether row satisfies every compiled predicate for the given -// claims — the per-subscriber, deferred-binding twin of ResolvedPermissions.RowVisible, -// and it must agree with it on every decision. A nil receiver (no policy applies) -// admits every row; a denied or bind-unsafe filter admits none (fail closed). -// numericCols is as in ResolvedPermissions.RowVisible. -func (c *CompiledRowFilter) RowVisible(row, claims map[string]any, numericCols map[string]bool) bool { - if c == nil { - return true - } - if !c.allowed { - return false - } - for _, pred := range c.preds { - if !pred.visible(row, claims, numericCols[pred.Column]) { - return false - } - } - return true -} - -// visible evaluates one compiled predicate against the row for the given claims. It -// binds the claim value at call time, then reuses compareScalar — the same comparison -// core as resolvedPredicate.matches — so the compiled and resolved paths can't drift on -// how values compare. Fails closed on an absent or uncomparable value, exactly as -// matches does. -func (cp compiledPredicate) visible(row, claims map[string]any, numeric bool) bool { - raw, ok := row[cp.Column] - if !ok { - return false // column not in the event ⇒ can't prove the row is allowed - } - if cp.Op == "in" { - for _, v := range cp.Value.bindIn(claims) { - if c, ok := compareScalar(raw, v, numeric); ok && c == 0 { - return true - } - } - return false - } - c, ok := compareScalar(raw, cp.Value.bindScalar(claims), numeric) - if !ok { - return false - } - switch cp.Op { - case "=": - return c == 0 - case "!=": - return c != 0 - case ">": - return c > 0 - case "<": - return c < 0 - default: - return false - } -} - // compareScalar compares an event value against a resolved filter value, returning // -1/0/+1 and ok=false when the comparison can't be made (a non-scalar event value, // or a numeric comparison whose operands don't parse as numbers). numeric selects diff --git a/internal/policy/rowfilter_test.go b/internal/policy/rowfilter_test.go index 8c7a804f..6e6103cc 100644 --- a/internal/policy/rowfilter_test.go +++ b/internal/policy/rowfilter_test.go @@ -1,11 +1,9 @@ package policy import ( - "strings" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // evalRowFilter builds a one-role/one-table policy carrying filter and returns the @@ -141,126 +139,3 @@ func TestRowVisible_MultiplePredicates_AllMustPass(t *testing.T) { assert.False(t, perms.RowVisible(map[string]any{"tenant_id": "acme", "amount": float64(9)}, num), "amount fails") assert.False(t, perms.RowVisible(map[string]any{"tenant_id": "globex", "amount": float64(250)}, num), "tenant fails") } - -// TestCompiledRowFilter_MatchesEvaluatePath pins the parity the compiled per-subscriber -// stream path depends on: for every (filter, claims, row) below, CompileRowFilter + -// RowVisible must return exactly what the query path (Evaluate → resolved predicates → -// matches) returns. Were the two to diverge, a subscriber could see rows the query path -// hides (or vice versa) — the -// row-level-security bug this path exists to prevent. The matrix exercises every operator, -// constant / whole-claim / mixed-template / space-padded / nested-path values, and -// missing columns, empty claim sets, and numeric-vs-lexicographic comparison. -func TestCompiledRowFilter_MatchesEvaluatePath(t *testing.T) { - t.Parallel() - filters := []map[string]Filter{ - {"tenant_id": {Eq: new("{{ jwt.tenant }}")}}, - {"tenant_id": {Neq: new("{{ jwt.tenant }}")}}, - {"amount": {Gt: new("100")}}, - {"amount": {Lt: new("{{ jwt.cap }}")}}, - {"status": {Eq: new("active")}}, // constant - {"region": {In: new("{{ jwt.regions }}")}}, // claim list - {"tag": {In: new("{{ jwt.tag }}")}}, // claim scalar - {"kind": {In: new("published")}}, // constant in-set - {"label": {Eq: new("v-{{ jwt.tier }}")}}, // mixed template - {"tenant_id": {Eq: new(" {{ jwt.tenant }} ")}}, // space-padded ⇒ stays a template - {"tenant_id": {Eq: new("{{ jwt.tenant }}"), Neq: new("blocked")}}, // two predicates on one column - {"org": {Eq: new("{{ jwt.org.id }}")}}, // nested claim path - } - claimsets := []map[string]any{ - nil, - {"tenant": "acme"}, - {"tenant": "acme", "cap": float64(500), "regions": []any{"us", "eu"}, "tag": "x", "tier": "pro", "org": map[string]any{"id": "o1"}}, - {"tenant": ""}, - {"tenant": "acme", "regions": []any{}}, // empty claim list - } - rows := []map[string]any{ - {"tenant_id": "acme", "amount": float64(250), "status": "active", "region": "us", "tag": "x", "kind": "published", "label": "v-pro", "org": "o1"}, - {"tenant_id": "globex", "amount": float64(9), "status": "off", "region": "apac", "tag": "y", "kind": "draft", "label": "v-free", "org": "o2"}, - {"amount": float64(500)}, // several columns missing ⇒ fail closed - {"tenant_id": "", "amount": "NaN", "org": "o1"}, - } - numeric := map[string]bool{"amount": true} - - for fi, f := range filters { - p := &Policy{Tables: map[string]TablePolicy{ - "t": {Select: map[string]RolePermissions{"viewer": {Filter: f}}}, - }} - compiled := CompileRowFilter(p, "viewer", "t", "select") - assert.Equalf(t, Evaluate(p, "viewer", "t", "select", nil).HasRowFilter(), compiled.HasRowFilter(), - "HasRowFilter parity, filter#%d", fi) - for _, claims := range claimsets { - resolved := Evaluate(p, "viewer", "t", "select", claims) - for ri, row := range rows { - want := resolved.RowVisible(row, numeric) - got := compiled.RowVisible(row, claims, numeric) - assert.Equalf(t, want, got, - "filter#%d claims=%v row#%d — compiled=%v resolved=%v", fi, claims, ri, got, want) - } - } - } -} - -// FuzzCompiledRowFilterParity is the property form of the matrix test above: for ANY -// operator, filter value, claim, and row the fuzzer synthesizes, the compiled -// per-subscriber stream path (CompileRowFilter + RowVisible) must agree with the query -// path (Evaluate + resolved predicates). The fuzzer probes the template-parsing edges -// where a divergence would hide — stray "{{", "jwt.", nested dots, whitespace, partial -// templates — that a hand-written matrix can't enumerate. The seed corpus also runs as a -// plain regression test under `go test` (no -fuzz needed). The column is fixed to a -// bind-safe name so the role always resolves as allowed; RowVisible parity is defined -// only for an allowed role (a denied role never reaches the per-subscriber check). -func FuzzCompiledRowFilterParity(f *testing.F) { - f.Add(uint8(0), "{{ jwt.x }}", "acme", "acme", true, false, false) // eq claim-ref, match - f.Add(uint8(1), " {{ jwt.x }} ", "acme", "acme", true, false, false) // space-padded ⇒ template - f.Add(uint8(2), "100", "", "250", true, true, false) // gt numeric constant - f.Add(uint8(3), "{{ jwt.x }}", "500", "250", true, true, false) // lt numeric claim - f.Add(uint8(4), "{{ jwt.x }}", "a,b,c", "b", true, false, true) // in claim-list - f.Add(uint8(0), "v-{{ jwt.x }}", "pro", "v-pro", true, false, false) // mixed template - f.Add(uint8(0), "{{ jwt.x }}", "acme", "acme", false, false, false) // column absent ⇒ fail closed - f.Add(uint8(0), "{{ jwt.a.b }}", "z", "z", true, false, false) // nested claim path - - f.Fuzz(func(t *testing.T, opSel uint8, filterVal, claimVal, rowVal string, colPresent, numeric, claimList bool) { - var filter Filter - switch opSel % 5 { - case 0: - filter.Eq = &filterVal - case 1: - filter.Neq = &filterVal - case 2: - filter.Gt = &filterVal - case 3: - filter.Lt = &filterVal - case 4: - filter.In = &filterVal - } - p := &Policy{Tables: map[string]TablePolicy{ - "t": {Select: map[string]RolePermissions{"r": {Filter: map[string]Filter{"col": filter}}}}, - }} - - var claim any = claimVal - if claimList { // exercise the _in []any path, not just a scalar claim - parts := strings.Split(claimVal, ",") - list := make([]any, len(parts)) - for i, s := range parts { - list[i] = s - } - claim = list - } - claims := map[string]any{"x": claim} - row := map[string]any{} - if colPresent { - row["col"] = rowVal - } - nc := map[string]bool{} - if numeric { - nc["col"] = true - } - - compiled := CompileRowFilter(p, "r", "t", "select") - resolved := Evaluate(p, "r", "t", "select", claims) - require.True(t, resolved.Allowed, "fixed bind-safe setup must resolve as allowed") - require.Equalf(t, resolved.RowVisible(row, nc), compiled.RowVisible(row, claims, nc), - "parity broke: op=%d filter=%q claim=%v(list=%v) row=%q(present=%v) numeric=%v", - opSel%5, filterVal, claim, claimList, rowVal, colPresent, numeric) - }) -} diff --git a/internal/stream/hub.go b/internal/stream/hub.go index c8af8390..934556f4 100644 --- a/internal/stream/hub.go +++ b/internal/stream/hub.go @@ -19,7 +19,7 @@ import ( // resolved against each subscriber's claims, so two subscribers of the same role can // be entitled to different rows. For a role that carries a row-filter, Broadcast // therefore keeps the shared column projection but evaluates row visibility PER -// subscriber (CompiledRowFilter.RowVisible) before delivering — closing the +// subscriber (ResolvedPermissions.RowVisible) before delivering — closing the // query/stream RLS drift in #319. Roles without a row-filter keep the pure // once-per-role fast path unchanged. See projectColumns. type Hub struct { @@ -161,16 +161,13 @@ func (h *Hub) Broadcast(topic string, raw []byte) { // shared, but whether each subscriber may see THIS row depends on its claims, so // evaluate visibility per subscriber. Predicates read the full event data (a // filter may key on a column the role can't SELECT), not the projected columns. - // The filter compiles once per bucket (claims-independent); only the claim - // binding inside RowVisible is per subscriber, so a filtered high-fan-out topic - // pays no per-subscriber predicate resolution. if !numericResolved { numericCols = h.numericCols(evt.TableName) numericResolved = true } - compiled := policy.CompileRowFilter(p, rb.role, evt.TableName, "select") for _, sub := range rb.bucket.Snapshot() { - if !compiled.RowVisible(evt.Data, sub.claims, numericCols) { + subPerms := policy.Evaluate(p, rb.role, evt.TableName, "select", sub.claims) + if !subPerms.RowVisible(evt.Data, numericCols) { continue // this row is filtered out for this subscriber } if !sub.Send(frame) { @@ -227,8 +224,8 @@ func (h *Hub) ReplayFrame(role string, claims map[string]any, raw []byte) (Frame return Frame{}, false } if perms.HasRowFilter() { - compiled := policy.CompileRowFilter(p, role, evt.TableName, "select") - if !compiled.RowVisible(evt.Data, claims, h.numericCols(evt.TableName)) { + subPerms := policy.Evaluate(p, role, evt.TableName, "select", claims) + if !subPerms.RowVisible(evt.Data, h.numericCols(evt.TableName)) { return Frame{}, false // this row is filtered out for these claims } } @@ -241,9 +238,8 @@ func (h *Hub) ReplayFrame(role string, claims map[string]any, raw []byte) (Frame // role+table policy entry (AllowColumns/DenyColumns), never from claims, so this // frame is byte-identical for every subscriber of a (role, table) — the shared // projection that lets one serialization serve a whole bucket. Row-level security is -// NOT applied here; when perms.HasRowFilter(), the caller compiles the role's filter -// once (policy.CompileRowFilter) and checks its RowVisible per subscriber with that -// subscriber's claims. ok=false means skip: the role can't +// NOT applied here; the caller checks perms.RowVisible per subscriber (with that +// subscriber's claims) when perms.HasRowFilter(). ok=false means skip: the role can't // read the table, or the payload is unusable. perms is nil for the legacy no-policy // passthrough (which has no row-filter). func projectColumns(p *policy.Policy, filter bool, role string, evt *ingest.EventMessage, raw []byte, decoded bool) (wire []byte, perms *policy.ResolvedPermissions, ok bool) { diff --git a/internal/stream/hub_test.go b/internal/stream/hub_test.go index 130b1765..f07d92cf 100644 --- a/internal/stream/hub_test.go +++ b/internal/stream/hub_test.go @@ -3,7 +3,6 @@ package stream import ( "context" "encoding/json" - "fmt" "strings" "sync" "testing" @@ -19,9 +18,8 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata" ) -// rawEvent marshals an EventMessage the way the ingest path publishes it. It takes -// testing.TB so both tests (*testing.T) and benchmarks (*testing.B) can build events. -func rawEvent(t testing.TB, table, ts string, data map[string]any) []byte { +// rawEvent marshals an EventMessage the way the ingest path publishes it. +func rawEvent(t *testing.T, table, ts string, data map[string]any) []byte { t.Helper() raw, err := json.Marshal(ingest.EventMessage{TableName: table, ReceivedTimestamp: ts, Data: data}) require.NoError(t, err) @@ -506,45 +504,6 @@ func TestWireFrame(t *testing.T) { } } -// BenchmarkBroadcast_RowFilteredFanout measures one Broadcast on a row-filtered topic -// as the subscriber count grows. The filter compiles once per bucket -// (policy.CompileRowFilter); each subscriber then pays only a claim-bound RowVisible on -// the full event, so allocations stay flat regardless of fan-out. This is the benchmark -// behind the O(subscribers) → O(1) allocation work on PR #381 — run it before/after a -// fan-out change to catch a regression back to per-subscriber resolution. It isolates -// the fan-out cost: the shared column projection is built once per event, and full -// outbound queues merely drop (a nil-metric no-op). -// -// go test ./internal/stream/ -run '^$' -bench BenchmarkBroadcast_RowFilteredFanout -benchmem -func BenchmarkBroadcast_RowFilteredFanout(b *testing.B) { - const topic = "ingest.clicks" - raw := rawEvent(b, "clicks", "2026-06-26T00:00:00Z", - map[string]any{"tenant_id": "acme", "page": "/a", "secret": "x"}) - - for _, n := range []int{100, 1_000, 10_000} { - b.Run(fmt.Sprintf("subscribers=%d", n), func(b *testing.B) { - hub := NewHub(policy.NewMemoryStore(rowFilterPolicy()), nil, nil) - // Half the subscribers share the event's tenant (row visible), half don't - // (row withheld); either way each pays the per-subscriber RowVisible check - // against the once-per-bucket compiled filter, which is the cost under - // measurement. - for i := range n { - sub := NewSubscriber() - tenant := "acme" - if i%2 == 1 { - tenant = "globex" - } - sub.SetClaims(map[string]any{"tenant": tenant}) - hub.Add(topic, "viewer", sub) - } - b.ReportAllocs() - for b.Loop() { - hub.Broadcast(topic, raw) - } - }) - } -} - // sumByName totals all datapoints of an Int64 sum instrument across kinds. func sumByName(rm metricdata.ResourceMetrics, name string) int64 { for _, sm := range rm.ScopeMetrics { diff --git a/tests/e2e/sdk/streaming.test.ts b/tests/e2e/sdk/streaming.test.ts index 1dc58e0f..2a1c9314 100644 --- a/tests/e2e/sdk/streaming.test.ts +++ b/tests/e2e/sdk/streaming.test.ts @@ -28,7 +28,7 @@ describe("Streaming", () => { // Explicitly allow the 'anon' role to SELECT (stream) from this suite's tables. // 'scoped' additionally carries a per-subscriber row filter — streamed rows are // limited to the caller's own country claim — so the SSE fan-out exercises the - // row-level-security path (CompileRowFilter → RowVisible) end to end, not just + // row-level-security path (ResolvedPermissions.RowVisible) end to end, not just // column projection. publicPolicy.tables[T.clicks].select = { ...(publicPolicy.tables[T.clicks].select || {}),