From 02713229cec0d58322391f74124dd1fd67a038ae Mon Sep 17 00:00:00 2001 From: Anton Zelenov Date: Fri, 31 Jul 2026 09:15:52 +0800 Subject: [PATCH 1/3] feat(analytics): inject flat tenant filter in the compiler's shared WHERE (#1967) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the single-tenant MVP no-op with a server-injected `tenant_id = ` predicate in the metric_results compiler — the one place every structured contract read compiles through. The predicate leads every observation read (`metric_where` / `shared_observation_where`) and both peer-cohort CTE reads, so a request scoped to tenant A cannot read tenant B's rows. - Carry the resolved tenant on `ValidatedMetricResultsRequest` (from the SecurityContext already passed to `validate_request`); never from client SQL. - Filter on `tenant_id`, the column the gold observation and cohort contract exposes (silver `insight_tenant_id` aliased to `tenant_id`); this surface is internally consistent, so the #1596 name drift does not affect it. Value is the raw tenant UUID — the representation the metric lineage stamps (no sipHash; that is identity-only, #1550). - Leave the legacy per-metric `query_ref` path unfiltered by design: its arbitrary FROM shapes make a flat predicate unsafe; its stale/misleading comments are corrected. - Extend the presentation PRD/DESIGN: mark the tenant-filter FR, NFR, and compiler component implemented; record the actual contract column name. Closes #1967 Part of #1803 Co-Authored-By: Claude Opus 4.8 Signed-off-by: Anton Zelenov --- .../domain/presentation-layer/specs/DESIGN.md | 8 +- docs/domain/presentation-layer/specs/PRD.md | 6 +- .../services/analytics/src/api/handlers.rs | 10 ++- .../src/domain/metric_results/batch.rs | 1 + .../src/domain/metric_results/builder.rs | 1 + .../src/domain/metric_results/compiler.rs | 78 +++++++++++++++---- .../src/domain/metric_results/validation.rs | 6 ++ 7 files changed, 86 insertions(+), 24 deletions(-) diff --git a/docs/domain/presentation-layer/specs/DESIGN.md b/docs/domain/presentation-layer/specs/DESIGN.md index 5a87956da..3c616d050 100644 --- a/docs/domain/presentation-layer/specs/DESIGN.md +++ b/docs/domain/presentation-layer/specs/DESIGN.md @@ -52,7 +52,7 @@ Requirements that significantly influence architecture decisions. | `cpt-presentation-fr-namespace` | New empty `presentation` database for new gold, saved-query results, and scratch; legacy gold left read-only in `insight` | | `cpt-presentation-fr-saved-query-crud` | The saved query (`presentation.queries` logically; the `saved_queries` table physically) is a SeaORM entity in the analytics **service database (MariaDB)**, like metric definitions; CRUD mutates that metadata, not ClickHouse. Only `/run` reaches ClickHouse — it reuses the existing read path and executes the stored SQL as `presentation_ro`, so no write grant on the contract is ever needed. Shipped (#1965) | | `cpt-presentation-fr-query-params` | Named parameters, `tenant` always injected from context (not client SQL), `period` supported | -| `cpt-presentation-fr-tenant-filter` | Literal `insight_tenant_id = ` injected in one place — the compiler's shared `WHERE` — replacing the no-op | +| `cpt-presentation-fr-tenant-filter` | Literal leading `tenant_id = ` injected in one place — the compiler's shared `WHERE` (and the peer-cohort CTE reads) — replacing the no-op. `tenant_id` is the column the gold observation and cohort contract exposes (silver's `insight_tenant_id`, aliased to `tenant_id` in gold); filtering on it sidesteps the #1596 name drift, which affects other tables, not this read surface. Shipped (#1967) | | `cpt-presentation-fr-contract-surface-doc` | Contract surface documented as the read boundary (silver and identity objects) | | `cpt-presentation-fr-contract-version-stamp` | Contract version stamp so presentation detects the surface it was built against | | `cpt-presentation-fr-query-console` | Single stable FE app on the saved-query API: author, list, run, render table / auto-chart | @@ -64,7 +64,7 @@ Requirements that significantly influence architecture decisions. | NFR ID | NFR Summary | Allocated To | Design Response | Verification Approach | |--------|-------------|--------------|-----------------|----------------------| | `cpt-presentation-nfr-source-immutability` | No presentation write reaches engineering-owned data | Single-SELECT gate + `presentation_ro` role | Two independent barriers: syntactic gate rejects non-`SELECT`; role grants forbid write/DDL on the contract | Adversarial SQL suite; verify no write/alter/drop on contract objects | -| `cpt-presentation-nfr-tenant-isolation` | No cross-tenant rows returned | Compiler shared `WHERE` | Server-injected literal tenant predicate the client SQL cannot widen | Cross-tenant isolation test returns zero rows | +| `cpt-presentation-nfr-tenant-isolation` | No cross-tenant rows returned | Compiler shared `WHERE` | Server-injected literal tenant predicate the client SQL cannot widen; sourced from `SecurityContext`, not the request body | Compiler unit tests assert the predicate and its bound value lead every observation and cohort read (#1967); cross-tenant e2e (#1359) returns zero rows | ### 1.3 Architecture Layers @@ -289,7 +289,7 @@ Plain CRUD over stored queries so a new analytics slice needs no engineering cha #### Metric Compiler (Tenant Filter) -- [ ] `p2` - **ID**: `cpt-presentation-component-metric-compiler` +- [x] `p2` - **ID**: `cpt-presentation-component-metric-compiler` ##### Why this component exists @@ -297,7 +297,7 @@ Builds contract SQL and owns the single shared `WHERE` where the tenant predicat ##### Responsibility scope -- Inject a literal `insight_tenant_id = ` on every contract read, sourced from request context. +- Inject a leading literal `tenant_id = ` on every contract read, sourced from the request's `SecurityContext` (carried on `ValidatedMetricResultsRequest`). `tenant_id` is the column the gold observation and cohort contract exposes; the value is the raw tenant UUID, the same representation the metric lineage stamps (no sipHash — that is identity-only). The predicate covers every observation read (`metric_where` / `shared_observation_where`) and both peer-cohort CTE reads. - Keep `FINAL` on silver `ReplacingMergeTree` reads. - Put `insight_tenant_id` first in `ORDER BY` for any new presentation gold that carries it. diff --git a/docs/domain/presentation-layer/specs/PRD.md b/docs/domain/presentation-layer/specs/PRD.md index c1e03bdb5..622451d4c 100644 --- a/docs/domain/presentation-layer/specs/PRD.md +++ b/docs/domain/presentation-layer/specs/PRD.md @@ -207,9 +207,9 @@ The system **MUST** support named query parameters, always injecting `tenant` fr #### Server-Injected Tenant Filter -- [ ] `p1` - **ID**: `cpt-presentation-fr-tenant-filter` +- [x] `p1` - **ID**: `cpt-presentation-fr-tenant-filter` -The system **MUST** inject a literal tenant predicate (`insight_tenant_id = `) server-side on every contract read, sourced from request context and not from client SQL. This **MUST** replace the current no-op filter. (#1967, coordinated with engineering #1829.) +The system **MUST** inject a literal tenant predicate (`tenant_id = `, the column the gold observation and cohort contract exposes) server-side on every contract read, sourced from request context and not from client SQL. This **MUST** replace the current no-op filter. (#1967, coordinated with engineering #1829.) **Rationale**: Every read is tenant-scoped; client SQL cannot widen it. @@ -285,7 +285,7 @@ The system **MUST** guarantee that no presentation-side operation can write, alt #### Tenant Read Isolation -- [ ] `p1` - **ID**: `cpt-presentation-nfr-tenant-isolation` +- [x] `p1` - **ID**: `cpt-presentation-nfr-tenant-isolation` Contract reads for tenant A **MUST NOT** return rows from tenant B, regardless of client SQL. diff --git a/src/backend/services/analytics/src/api/handlers.rs b/src/backend/services/analytics/src/api/handlers.rs index 967521e23..38bd3e428 100644 --- a/src/backend/services/analytics/src/api/handlers.rs +++ b/src/backend/services/analytics/src/api/handlers.rs @@ -261,8 +261,7 @@ async fn execute_metric_query( // 4. Build ClickHouse query from structured metric fields. // - // The engine always controls FROM and WHERE — insight_tenant_id is - // always injected for tenant isolation. Admins never control WHERE. + // The engine always controls FROM and WHERE; admins never control WHERE. // // Person ID resolution: if identity_url is configured, person_ids from // $filter would be resolved to source aliases via the Identity API. @@ -295,8 +294,11 @@ async fn execute_metric_query( _ => select_expr, }; - // MVP: single tenant — skip tenant isolation filter. - // TODO: re-enable for multi-tenant: WHERE insight_tenant_id = ? + // This legacy path runs arbitrary DB-stored `query_ref` FROM shapes + // (subqueries, bare bronze tables), not the uniform observation contract, so + // a flat `tenant_id = ?` cannot be injected safely here. Tenant isolation for + // the structured read path lives in one place — the metric_results compiler's + // shared WHERE. let mut params: Vec = vec![]; // If the FROM clause is a subquery, we inject the metric_date range INSIDE the diff --git a/src/backend/services/analytics/src/domain/metric_results/batch.rs b/src/backend/services/analytics/src/domain/metric_results/batch.rs index ccc1f9000..fe624d420 100644 --- a/src/backend/services/analytics/src/domain/metric_results/batch.rs +++ b/src/backend/services/analytics/src/domain/metric_results/batch.rs @@ -430,6 +430,7 @@ mod tests { fn request(metrics: Vec) -> ValidatedMetricResultsRequest { ValidatedMetricResultsRequest { + tenant_id: uuid::Uuid::from_u128(0x1967), entity_type: "person".to_owned(), entity_ids: vec!["a@x.io".to_owned()], from: NaiveDate::from_ymd_opt(2026, 1, 1).unwrap_or_default(), diff --git a/src/backend/services/analytics/src/domain/metric_results/builder.rs b/src/backend/services/analytics/src/domain/metric_results/builder.rs index 7551bda6a..718486203 100644 --- a/src/backend/services/analytics/src/domain/metric_results/builder.rs +++ b/src/backend/services/analytics/src/domain/metric_results/builder.rs @@ -446,6 +446,7 @@ mod tests { fn request(entity_ids: Vec<&str>, from: &str, to: &str) -> ValidatedMetricResultsRequest { ValidatedMetricResultsRequest { + tenant_id: uuid::Uuid::from_u128(0x1967), entity_type: "person".to_owned(), entity_ids: entity_ids.into_iter().map(str::to_owned).collect(), from: match NaiveDate::parse_from_str(from, "%Y-%m-%d") { diff --git a/src/backend/services/analytics/src/domain/metric_results/compiler.rs b/src/backend/services/analytics/src/domain/metric_results/compiler.rs index 134532d60..80c0b8c90 100644 --- a/src/backend/services/analytics/src/domain/metric_results/compiler.rs +++ b/src/backend/services/analytics/src/domain/metric_results/compiler.rs @@ -508,9 +508,11 @@ pub(crate) fn compile_peer_batch_query( filters: &[ValidatedDimensionFilter], ) -> CompiledQuery { let mut params = Vec::new(); + params.push(req.tenant_id.to_string()); params.push(req.entity_type.clone()); params.push(cohort_key.to_owned()); params.extend(req.entity_ids.iter().cloned()); + params.push(req.tenant_id.to_string()); params.push(req.entity_type.clone()); params.push(cohort_key.to_owned()); let value_selects = item_value_selects(defs, &mut params, period_alias); @@ -573,7 +575,7 @@ pub(crate) fn compile_peer_batch_query( entity_id, cohort_id FROM {cohort_table} - WHERE entity_type = ? + WHERE tenant_id = ? AND entity_type = ? AND cohort_key = ? AND entity_id IN ({entities}) AND cohort_id IS NOT NULL @@ -583,7 +585,7 @@ pub(crate) fn compile_peer_batch_query( entity_id, cohort_id FROM {cohort_table} - WHERE entity_type = ? + WHERE tenant_id = ? AND entity_type = ? AND cohort_key = ? AND cohort_id IN (SELECT cohort_id FROM targets) ), @@ -756,6 +758,7 @@ fn shared_observation_where( filters: &[ValidatedDimensionFilter], params: &mut Vec, ) -> String { + params.push(req.tenant_id.to_string()); params.push(req.entity_type.clone()); params.push(req.from.to_string()); params.push(req.to.to_string()); @@ -766,7 +769,7 @@ fn shared_observation_where( } let pair_placeholders = vec!["(?, ?)"; pairs.len()].join(", "); let mut where_clause = format!( - "entity_type = ? AND metric_date >= toDate(?) AND metric_date <= toDate(?) AND (source_key, measure_key) IN ({pair_placeholders})" + "tenant_id = ? AND entity_type = ? AND metric_date >= toDate(?) AND metric_date <= toDate(?) AND (source_key, measure_key) IN ({pair_placeholders})" ); where_clause.push_str(&dimension_filter_where(filters, params)); where_clause @@ -819,21 +822,21 @@ fn batch_observation_table(defs: &[&MetricDefinition]) -> String { observation_table(def.observation_relation()) } -// No tenant_id predicate: warehouse tenant isolation is not implemented -// platform-wide (the legacy query engine also queries without it), and the -// control-plane tenant UUID has no defined mapping to the warehouse -// tenant_id strings stamped at ingestion. The observation and cohort -// contracts keep the tenant_id column so isolation can be added here in one -// place once the platform defines that mapping. +// INVARIANT: every observation read leads with `tenant_id = ?`, bound from the +// request's SecurityContext (never client SQL), so a request scoped to tenant A +// cannot read tenant B's rows. `tenant_id` is the column the gold observation +// and cohort contract exposes; the value is the raw tenant UUID, the same +// representation the metric lineage stamps. The placeholder is first here and +// its value first in `metric_where_params` — keep the two in lockstep. fn metric_where(def: &MetricDefinition) -> &'static str { match &def.spec { ComputationSpec::Sum { .. } | ComputationSpec::Median { .. } | ComputationSpec::DistinctCount { .. } => { - "source_key = ? AND entity_type = ? AND metric_date >= toDate(?) AND metric_date <= toDate(?) AND measure_key = ?" + "tenant_id = ? AND source_key = ? AND entity_type = ? AND metric_date >= toDate(?) AND metric_date <= toDate(?) AND measure_key = ?" } ComputationSpec::Ratio { .. } => { - "source_key = ? AND entity_type = ? AND metric_date >= toDate(?) AND metric_date <= toDate(?) AND measure_key IN (?, ?)" + "tenant_id = ? AND source_key = ? AND entity_type = ? AND metric_date >= toDate(?) AND metric_date <= toDate(?) AND measure_key IN (?, ?)" } } } @@ -865,6 +868,7 @@ fn metric_where_params(def: &MetricDefinition, req: &ValidatedMetricResultsReque ComputationSpec::Sum { value } | ComputationSpec::Median { value } | ComputationSpec::DistinctCount { value } => vec![ + req.tenant_id.to_string(), value.source_key.clone(), req.entity_type.clone(), req.from.to_string(), @@ -876,6 +880,7 @@ fn metric_where_params(def: &MetricDefinition, req: &ValidatedMetricResultsReque denominator, .. } => vec![ + req.tenant_id.to_string(), numerator.source_key.clone(), req.entity_type.clone(), req.from.to_string(), @@ -1074,8 +1079,12 @@ mod tests { } } + const TEST_TENANT: uuid::Uuid = uuid::Uuid::from_u128(0x1967); + const TEST_TENANT_STR: &str = "00000000-0000-0000-0000-000000001967"; + fn request() -> ValidatedMetricResultsRequest { ValidatedMetricResultsRequest { + tenant_id: TEST_TENANT, entity_type: "person".to_owned(), entity_ids: vec!["a@x.io".to_owned(), "b@x.io".to_owned()], from: NaiveDate::from_ymd_opt(2026, 1, 1).unwrap_or_default(), @@ -1089,7 +1098,11 @@ mod tests { let (sum, ratio) = (sum_metric(), ratio_metric()); let query = compile_period_batch_query(&[&sum, &ratio], &request(), &[]); assert!(query.sql.contains("FROM insight.ai_metric_observations")); - assert!(!query.sql.contains("tenant_id")); + assert!( + query + .sql + .contains("WHERE tenant_id = ? AND entity_type = ?") + ); assert!(query.sql.contains("AS m0")); assert!(query.sql.contains("AS m1")); assert!( @@ -1115,7 +1128,8 @@ mod tests { "accepted_edit_actions", "ai_usage", "tool_use_offered", - // shared scope + // shared scope (tenant predicate leads) + TEST_TENANT_STR, "person", "2026-01-01", "2026-01-31", @@ -1154,6 +1168,7 @@ mod tests { "accepted_edit_actions", "ai_usage", "tool_use_offered", + TEST_TENANT_STR, "person", "2026-01-01", "2026-01-31", @@ -1167,6 +1182,36 @@ mod tests { ); } + #[test] + fn tenant_predicate_leads_and_binds_context_tenant_on_every_contract_read() { + let sum = sum_metric(); + + let ts = compile_timeseries_query(&sum, &request(), Bucket::Day, &[], &[], None); + assert!(ts.sql.contains("WHERE tenant_id = ?"), "timeseries read"); + assert_eq!(ts.params.first().map(String::as_str), Some(TEST_TENANT_STR)); + assert_eq!(ts.sql.matches('?').count(), ts.params.len()); + + let rank = compile_group_ranking_query(&sum, &request(), &["tool".to_owned()], &[], 5); + assert!(rank.sql.contains("WHERE tenant_id = ?"), "ranking read"); + assert_eq!( + rank.params.first().map(String::as_str), + Some(TEST_TENANT_STR) + ); + + // The peer query reads the contract three times (targets, cohort, + // metric_values); each must carry the tenant predicate and its value. + let peer = compile_peer_batch_query(&[&sum], &request(), "org_unit", &[]); + assert_eq!(peer.sql.matches("tenant_id = ?").count(), 3); + assert_eq!( + peer.params + .iter() + .filter(|p| p.as_str() == TEST_TENANT_STR) + .count(), + 3 + ); + assert_eq!(peer.sql.matches('?').count(), peer.params.len()); + } + #[test] fn timeseries_query_uses_bucket_expression() { for (bucket, expr) in [ @@ -1318,14 +1363,21 @@ mod tests { assert_eq!( query.params, vec![ + // targets CTE (tenant predicate leads every read) + TEST_TENANT_STR, "person", "org_unit", "a@x.io", "b@x.io", + // cohort CTE + TEST_TENANT_STR, "person", "org_unit", + // item value selects "ai_usage", "accepted_lines", + // metric_values shared scope + TEST_TENANT_STR, "person", "2026-01-01", "2026-01-31", diff --git a/src/backend/services/analytics/src/domain/metric_results/validation.rs b/src/backend/services/analytics/src/domain/metric_results/validation.rs index 2007569c5..6445d1c9f 100644 --- a/src/backend/services/analytics/src/domain/metric_results/validation.rs +++ b/src/backend/services/analytics/src/domain/metric_results/validation.rs @@ -31,6 +31,7 @@ pub(crate) const HISTOGRAM_BINS: usize = 10; #[derive(Debug)] pub struct ValidatedMetricResultsRequest { + pub tenant_id: Uuid, pub entity_type: String, pub entity_ids: Vec, pub from: NaiveDate, @@ -171,6 +172,7 @@ pub async fn validate_request( } let validated = ValidatedMetricResultsRequest { + tenant_id, entity_type, entity_ids, from, @@ -1122,6 +1124,7 @@ mod tests { fn projected_view_limit_counts_timeseries_buckets() { let def = sum_definition(vec![]); let validated = ValidatedMetricResultsRequest { + tenant_id: Uuid::nil(), entity_type: "person".to_owned(), entity_ids: (0..100).map(|i| format!("p{i}@x.io")).collect(), from: day("2026-01-01"), @@ -1152,6 +1155,7 @@ mod tests { }), }; let validated = ValidatedMetricResultsRequest { + tenant_id: Uuid::nil(), entity_type: "person".to_owned(), entity_ids: vec!["a@x.io".to_owned()], from: day("2025-07-21"), @@ -1182,6 +1186,7 @@ mod tests { fn projected_view_limit_counts_histogram_bins() { // 501 entities × 10 bins > 5000 projected rows. let validated = ValidatedMetricResultsRequest { + tenant_id: Uuid::nil(), entity_type: "person".to_owned(), entity_ids: (0..501).map(|i| format!("p{i}@x.io")).collect(), from: day("2026-01-01"), @@ -1199,6 +1204,7 @@ mod tests { fn projected_view_limit_allows_small_requests() { let def = sum_definition(vec![]); let validated = ValidatedMetricResultsRequest { + tenant_id: Uuid::nil(), entity_type: "person".to_owned(), entity_ids: vec!["a@x.io".to_owned()], from: day("2026-01-01"), From 6b6336032e358a59a7db35cc103e902b2c0ffc7b Mon Sep 17 00:00:00 2001 From: Anton Zelenov Date: Fri, 31 Jul 2026 11:38:43 +0800 Subject: [PATCH 2/3] docs(presentation): scope tenant-isolation claims to the metric_results path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review: the compiler filter (#1967) covers only the structured metric_results reads, not the legacy execute_metric_query query_ref path, which runs arbitrary FROM shapes and stays unscoped. Narrow the FR/NFR completion claims accordingly — the compiler component is done, but the umbrella "every contract read" FR and the isolation NFR remain open until the legacy path is protected or restricted to tenant-safe sources. Document the legacy path as an explicit boundary outside the guarantee. Signed-off-by: Anton Zelenov --- docs/domain/presentation-layer/specs/DESIGN.md | 7 ++++--- docs/domain/presentation-layer/specs/PRD.md | 8 ++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/domain/presentation-layer/specs/DESIGN.md b/docs/domain/presentation-layer/specs/DESIGN.md index 3c616d050..baeacddf8 100644 --- a/docs/domain/presentation-layer/specs/DESIGN.md +++ b/docs/domain/presentation-layer/specs/DESIGN.md @@ -52,7 +52,7 @@ Requirements that significantly influence architecture decisions. | `cpt-presentation-fr-namespace` | New empty `presentation` database for new gold, saved-query results, and scratch; legacy gold left read-only in `insight` | | `cpt-presentation-fr-saved-query-crud` | The saved query (`presentation.queries` logically; the `saved_queries` table physically) is a SeaORM entity in the analytics **service database (MariaDB)**, like metric definitions; CRUD mutates that metadata, not ClickHouse. Only `/run` reaches ClickHouse — it reuses the existing read path and executes the stored SQL as `presentation_ro`, so no write grant on the contract is ever needed. Shipped (#1965) | | `cpt-presentation-fr-query-params` | Named parameters, `tenant` always injected from context (not client SQL), `period` supported | -| `cpt-presentation-fr-tenant-filter` | Literal leading `tenant_id = ` injected in one place — the compiler's shared `WHERE` (and the peer-cohort CTE reads) — replacing the no-op. `tenant_id` is the column the gold observation and cohort contract exposes (silver's `insight_tenant_id`, aliased to `tenant_id` in gold); filtering on it sidesteps the #1596 name drift, which affects other tables, not this read surface. Shipped (#1967) | +| `cpt-presentation-fr-tenant-filter` | Literal leading `tenant_id = ` injected in one place — the compiler's shared `WHERE` (and the peer-cohort CTE reads) — replacing the no-op. `tenant_id` is the column the gold observation and cohort contract exposes (silver's `insight_tenant_id`, aliased to `tenant_id` in gold); filtering on it sidesteps the #1596 name drift, which affects other tables, not this read surface. Shipped for the structured `metric_results` read path (#1967). The legacy per-metric `query_ref` path (`execute_metric_query`) remains unscoped and is explicitly outside this guarantee until protected — see the component boundaries below. | | `cpt-presentation-fr-contract-surface-doc` | Contract surface documented as the read boundary (silver and identity objects) | | `cpt-presentation-fr-contract-version-stamp` | Contract version stamp so presentation detects the surface it was built against | | `cpt-presentation-fr-query-console` | Single stable FE app on the saved-query API: author, list, run, render table / auto-chart | @@ -64,7 +64,7 @@ Requirements that significantly influence architecture decisions. | NFR ID | NFR Summary | Allocated To | Design Response | Verification Approach | |--------|-------------|--------------|-----------------|----------------------| | `cpt-presentation-nfr-source-immutability` | No presentation write reaches engineering-owned data | Single-SELECT gate + `presentation_ro` role | Two independent barriers: syntactic gate rejects non-`SELECT`; role grants forbid write/DDL on the contract | Adversarial SQL suite; verify no write/alter/drop on contract objects | -| `cpt-presentation-nfr-tenant-isolation` | No cross-tenant rows returned | Compiler shared `WHERE` | Server-injected literal tenant predicate the client SQL cannot widen; sourced from `SecurityContext`, not the request body | Compiler unit tests assert the predicate and its bound value lead every observation and cohort read (#1967); cross-tenant e2e (#1359) returns zero rows | +| `cpt-presentation-nfr-tenant-isolation` | No cross-tenant rows returned from the structured `metric_results` reads | Compiler shared `WHERE` | Server-injected literal tenant predicate the client SQL cannot widen; sourced from `SecurityContext`, not the request body | Compiler unit tests assert the predicate and its bound value lead every observation and cohort read (#1967); cross-tenant e2e (#1359) returns zero rows. Not yet met for the legacy `execute_metric_query` path, which stays outside the guarantee until protected. | ### 1.3 Architecture Layers @@ -297,7 +297,7 @@ Builds contract SQL and owns the single shared `WHERE` where the tenant predicat ##### Responsibility scope -- Inject a leading literal `tenant_id = ` on every contract read, sourced from the request's `SecurityContext` (carried on `ValidatedMetricResultsRequest`). `tenant_id` is the column the gold observation and cohort contract exposes; the value is the raw tenant UUID, the same representation the metric lineage stamps (no sipHash — that is identity-only). The predicate covers every observation read (`metric_where` / `shared_observation_where`) and both peer-cohort CTE reads. +- Inject a leading literal `tenant_id = ` on every read the compiler emits, sourced from the request's `SecurityContext` (carried on `ValidatedMetricResultsRequest`). `tenant_id` is the column the gold observation and cohort contract exposes; the value is the raw tenant UUID, the same representation the metric lineage stamps (no sipHash — that is identity-only). The predicate covers every observation read (`metric_where` / `shared_observation_where`) and both peer-cohort CTE reads. - Keep `FINAL` on silver `ReplacingMergeTree` reads. - Put `insight_tenant_id` first in `ORDER BY` for any new presentation gold that carries it. @@ -305,6 +305,7 @@ Builds contract SQL and owns the single shared `WHERE` where the tenant predicat - Does NOT read the tenant value from client SQL. - Does NOT implement subtree/hierarchy scoping in Phase A (deferred to the benchmark). +- Does NOT cover the legacy per-metric `query_ref` path (`execute_metric_query`, `/v1/metrics/{id}/query` and `/v1/metrics/queries`). That path runs arbitrary DB-stored `FROM` shapes (subqueries, bare bronze tables) where a flat `tenant_id = ?` cannot be injected safely, so it stays unscoped and outside the isolation guarantee until it is restricted to tenant-safe sources or given per-query enforcement. It predates this component; #1967 does not widen its exposure. ##### Related components (by ID) diff --git a/docs/domain/presentation-layer/specs/PRD.md b/docs/domain/presentation-layer/specs/PRD.md index 622451d4c..7254e0dfb 100644 --- a/docs/domain/presentation-layer/specs/PRD.md +++ b/docs/domain/presentation-layer/specs/PRD.md @@ -207,10 +207,12 @@ The system **MUST** support named query parameters, always injecting `tenant` fr #### Server-Injected Tenant Filter -- [x] `p1` - **ID**: `cpt-presentation-fr-tenant-filter` +- [ ] `p1` - **ID**: `cpt-presentation-fr-tenant-filter` The system **MUST** inject a literal tenant predicate (`tenant_id = `, the column the gold observation and cohort contract exposes) server-side on every contract read, sourced from request context and not from client SQL. This **MUST** replace the current no-op filter. (#1967, coordinated with engineering #1829.) +**Status**: Shipped for the structured `metric_results` read path (#1967). The legacy per-metric `query_ref` path (`execute_metric_query`) is not yet scoped and stays outside the guarantee until it is restricted to tenant-safe sources or given per-query enforcement; the requirement stays open until all exposed contract-read paths enforce tenant scope. + **Rationale**: Every read is tenant-scoped; client SQL cannot widen it. **Actors**: `cpt-presentation-actor-analytics-svc`, `cpt-presentation-actor-analyst` @@ -285,12 +287,14 @@ The system **MUST** guarantee that no presentation-side operation can write, alt #### Tenant Read Isolation -- [x] `p1` - **ID**: `cpt-presentation-nfr-tenant-isolation` +- [ ] `p1` - **ID**: `cpt-presentation-nfr-tenant-isolation` Contract reads for tenant A **MUST NOT** return rows from tenant B, regardless of client SQL. **Threshold**: 0 cross-tenant rows returned in isolation testing. +**Status**: Met for the structured `metric_results` read path (#1967, verified by compiler unit tests and the #1359 e2e). Not yet met for the legacy `execute_metric_query` path; the NFR stays open until isolation testing covers every exposed contract-read path. + **Rationale**: Multi-tenant SaaS compliance requirement. ### 6.2 NFR Exclusions From e39f06a239cea96aef75ef11fa74017d35866d9c Mon Sep 17 00:00:00 2001 From: Anton Zelenov Date: Fri, 31 Jul 2026 13:07:12 +0800 Subject: [PATCH 3/3] test(e2e): stamp bronze fixtures with the harness tenant The compiler now binds the request's SecurityContext tenant as a leading tenant_id predicate on every gold read, so seeded bronze rows must carry the tenant the harness's gateway JWT is scoped to. The nil-UUID stamp matched nothing and every metric smoke test read zero rows. Signed-off-by: Anton Zelenov --- src/ingestion/tests/e2e/metrics/templates/bitbucket_git.yaml | 2 +- .../tests/e2e/metrics/templates/chatgpt_team_chat.yaml | 2 +- .../tests/e2e/metrics/templates/chatgpt_team_codex.yaml | 2 +- .../tests/e2e/metrics/templates/claude_enterprise_users.yaml | 2 +- .../tests/e2e/metrics/templates/claude_team_usage.yaml | 2 +- src/ingestion/tests/e2e/metrics/templates/cursor_events.yaml | 2 +- src/ingestion/tests/e2e/metrics/templates/cursor_usage.yaml | 2 +- src/ingestion/tests/e2e/metrics/templates/git_activity.yaml | 2 +- src/ingestion/tests/e2e/metrics/templates/jira_task.yaml | 4 ++-- src/ingestion/tests/e2e/metrics/templates/m365_email.yaml | 2 +- src/ingestion/tests/e2e/metrics/templates/m365_onedrive.yaml | 2 +- src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml | 2 +- .../tests/e2e/metrics/templates/outline_wiki_comments.yaml | 2 +- .../tests/e2e/metrics/templates/outline_wiki_pages.yaml | 2 +- src/ingestion/tests/e2e/metrics/templates/people.yaml | 2 +- .../tests/e2e/metrics/templates/wiki_page_versions.yaml | 2 +- 16 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/ingestion/tests/e2e/metrics/templates/bitbucket_git.yaml b/src/ingestion/tests/e2e/metrics/templates/bitbucket_git.yaml index 1f1b596e1..56e07b7d1 100644 --- a/src/ingestion/tests/e2e/metrics/templates/bitbucket_git.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/bitbucket_git.yaml @@ -10,7 +10,7 @@ templates: _airbyte_extracted_at: "2026-10-02T00:00:00" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: bitbucket-test data_source: insight_bitbucket_cloud record_type: item diff --git a/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_chat.yaml b/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_chat.yaml index 712f5b7cb..a34d2c03d 100644 --- a/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_chat.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_chat.yaml @@ -10,7 +10,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00Z" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "chatgpt-team-test" unique_key: null collected_at: "2026-01-05T00:00:00Z" diff --git a/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_codex.yaml b/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_codex.yaml index 29a804868..317640416 100644 --- a/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_codex.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/chatgpt_team_codex.yaml @@ -10,7 +10,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00Z" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "chatgpt-team-test" unique_key: null collected_at: "2026-01-05T00:00:00Z" diff --git a/src/ingestion/tests/e2e/metrics/templates/claude_enterprise_users.yaml b/src/ingestion/tests/e2e/metrics/templates/claude_enterprise_users.yaml index 7b001b435..b29728407 100644 --- a/src/ingestion/tests/e2e/metrics/templates/claude_enterprise_users.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/claude_enterprise_users.yaml @@ -12,7 +12,7 @@ templates: _airbyte_meta: "{}" _airbyte_generation_id: 0 unique_key: null - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "claude-enterprise-test" date: null user_id: null diff --git a/src/ingestion/tests/e2e/metrics/templates/claude_team_usage.yaml b/src/ingestion/tests/e2e/metrics/templates/claude_team_usage.yaml index 6fee6d0f3..7e5149cd5 100644 --- a/src/ingestion/tests/e2e/metrics/templates/claude_team_usage.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/claude_team_usage.yaml @@ -9,7 +9,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00Z" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "claude-team-test" unique_key: null collected_at: null diff --git a/src/ingestion/tests/e2e/metrics/templates/cursor_events.yaml b/src/ingestion/tests/e2e/metrics/templates/cursor_events.yaml index dacfb8088..82eb549df 100644 --- a/src/ingestion/tests/e2e/metrics/templates/cursor_events.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/cursor_events.yaml @@ -19,7 +19,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00Z" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "cursor-test" unique_key: null userEmail: null diff --git a/src/ingestion/tests/e2e/metrics/templates/cursor_usage.yaml b/src/ingestion/tests/e2e/metrics/templates/cursor_usage.yaml index d8f7cb8e7..a830e711d 100644 --- a/src/ingestion/tests/e2e/metrics/templates/cursor_usage.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/cursor_usage.yaml @@ -13,7 +13,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00Z" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "cursor-test" unique_key: null userId: null diff --git a/src/ingestion/tests/e2e/metrics/templates/git_activity.yaml b/src/ingestion/tests/e2e/metrics/templates/git_activity.yaml index fca237452..4eeca0b46 100644 --- a/src/ingestion/tests/e2e/metrics/templates/git_activity.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/git_activity.yaml @@ -4,7 +4,7 @@ templates: _airbyte_extracted_at: "2026-10-02T00:00:00Z" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: git-test unique_key: null commit: diff --git a/src/ingestion/tests/e2e/metrics/templates/jira_task.yaml b/src/ingestion/tests/e2e/metrics/templates/jira_task.yaml index 1bd992390..783a0bb2f 100644 --- a/src/ingestion/tests/e2e/metrics/templates/jira_task.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/jira_task.yaml @@ -34,7 +34,7 @@ templates: jira_user: unique_key: null source_id: jira-test - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" account_id: null email: null display_name: null @@ -79,7 +79,7 @@ templates: jira_history: unique_key: null source_id: jira-test - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" id_readable: null changelog_id: null created_at: "" diff --git a/src/ingestion/tests/e2e/metrics/templates/m365_email.yaml b/src/ingestion/tests/e2e/metrics/templates/m365_email.yaml index 588736165..fbf2cbd0a 100644 --- a/src/ingestion/tests/e2e/metrics/templates/m365_email.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/m365_email.yaml @@ -14,7 +14,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "m365-test" unique_key: null userPrincipalName: null diff --git a/src/ingestion/tests/e2e/metrics/templates/m365_onedrive.yaml b/src/ingestion/tests/e2e/metrics/templates/m365_onedrive.yaml index 59d6e9242..f024d25c1 100644 --- a/src/ingestion/tests/e2e/metrics/templates/m365_onedrive.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/m365_onedrive.yaml @@ -7,7 +7,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "m365-test" unique_key: null userPrincipalName: null diff --git a/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml b/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml index c616726cf..f00cadb05 100644 --- a/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml @@ -12,7 +12,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "m365-test" unique_key: null userPrincipalName: null diff --git a/src/ingestion/tests/e2e/metrics/templates/outline_wiki_comments.yaml b/src/ingestion/tests/e2e/metrics/templates/outline_wiki_comments.yaml index fb5ee097d..1777d16bc 100644 --- a/src/ingestion/tests/e2e/metrics/templates/outline_wiki_comments.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/outline_wiki_comments.yaml @@ -6,7 +6,7 @@ templates: _airbyte_meta: "{}" _airbyte_generation_id: 0 unique_key: null - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "wiki-test" page_id: null comment_id: null diff --git a/src/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yaml b/src/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yaml index d740438d7..f38e9bec6 100644 --- a/src/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yaml @@ -6,7 +6,7 @@ templates: _airbyte_meta: "{}" _airbyte_generation_id: 0 unique_key: null - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "wiki-test" page_id: null space_id: null diff --git a/src/ingestion/tests/e2e/metrics/templates/people.yaml b/src/ingestion/tests/e2e/metrics/templates/people.yaml index fac18c4fc..661e76c0c 100644 --- a/src/ingestion/tests/e2e/metrics/templates/people.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/people.yaml @@ -14,7 +14,7 @@ templates: _airbyte_extracted_at: "2026-01-05T00:00:00" _airbyte_meta: "{}" _airbyte_generation_id: 0 - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: "bamboohr-test" id: null unique_key: null diff --git a/src/ingestion/tests/e2e/metrics/templates/wiki_page_versions.yaml b/src/ingestion/tests/e2e/metrics/templates/wiki_page_versions.yaml index 7ff86a75a..c52eeb45e 100644 --- a/src/ingestion/tests/e2e/metrics/templates/wiki_page_versions.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/wiki_page_versions.yaml @@ -5,7 +5,7 @@ templates: _airbyte_meta: "{}" _airbyte_generation_id: 0 unique_key: null - tenant_id: "00000000-0000-0000-0000-000000000000" + tenant_id: "11111111-1111-1111-1111-111111111111" source_id: wiki-test page_id: null revision_id: null