feat(analytics): inject flat tenant filter in the compiler's shared WHERE (#1967)

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 = <ctx.tenant>` injected in one place — the compiler's shared `WHERE` — replacing the no-op |
+| `cpt-presentation-fr-tenant-filter` | Literal leading `tenant_id = <ctx.tenant>` 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 | Cross-tenant isolation test 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
 
@@ -289,22 +289,23 @@ 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
 
 Builds contract SQL and owns the single shared `WHERE` where the tenant predicate is injected, replacing the no-op left from the single-tenant MVP.
 
 ##### Responsibility scope
 
-- Inject a literal `insight_tenant_id = <ctx.tenant>` on every contract read, sourced from request context.
+- Inject a leading literal `tenant_id = <ctx.tenant>` 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.
 
 ##### Responsibility boundaries
 
 - 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)
 

docs/domain/presentation-layer/specs/PRD.md

@@ -209,7 +209,9 @@ The system **MUST** support named query parameters, always injecting `tenant` fr
 
 - [ ] `p1` - **ID**: `cpt-presentation-fr-tenant-filter`
 
-The system **MUST** inject a literal tenant predicate (`insight_tenant_id = <ctx.tenant>`) 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 = <ctx.tenant>`, 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.
 
@@ -291,6 +293,8 @@ Contract reads for tenant A **MUST NOT** return rows from tenant B, regardless o
 
 **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

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<String> = vec![];
 
     // If the FROM clause is a subquery, we inject the metric_date range INSIDE the

src/backend/services/analytics/src/domain/metric_results/batch.rs

@@ -430,6 +430,7 @@ mod tests {
 
     fn request(metrics: Vec<ValidatedMetricRequest>) -> 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(),

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") {

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>,
 ) -> 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",

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<String>,
     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"),

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

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"