fix: address cube tenant isolation review

apps/web/modules/ee/analysis/api/lib/cube-client.test.ts

@@ -122,6 +122,21 @@ describe("executeTenantScopedQuery", () => {
 
     const cubejs = await getCubeJsMock();
     expect(cubejs).not.toHaveBeenCalled();
+    expect(mockQueueAuditEventWithoutRequest).toHaveBeenCalledWith(
+      expect.objectContaining({
+        action: "queried",
+        targetType: "cubeQuery",
+        status: "failure",
+        newObject: expect.objectContaining({
+          tenantId: "workspace-1",
+          query: expect.objectContaining({
+            filterMembers: ["FeedbackRecords.tenantId"],
+            filterCount: 1,
+          }),
+          errorName: "Error",
+        }),
+      })
+    );
   });
 
   test("preserves API URL when it already contains /cubejs-api/v1", async () => {

apps/web/modules/ee/analysis/api/lib/cube-client.ts

@@ -1,5 +1,6 @@
 import "server-only";
 import cubejs, { type Query } from "@cubejs-client/core";
+import { randomUUID } from "node:crypto";
 import { logger } from "@formbricks/logger";
 import type { TChartQuery } from "@formbricks/types/analysis";
 import { queueAuditEventWithoutRequest } from "@/modules/ee/audit-logs/lib/handler";
@@ -57,10 +58,14 @@ const queueCubeQueryAuditEvent = ({
 };
 
 export async function executeTenantScopedQuery(input: TScopedCubeQueryInput) {
-  validateCubeQueryMembers(input.query);
+  try {
+    validateCubeQueryMembers(input.query);
+  } catch (error) {
+    queueCubeQueryAuditEvent({ error, input, requestId: randomUUID(), status: "failure" });
+    throw error;
+  }
 
   const tenantScope = {
-    tenantId: input.workspaceId,
     workspaceId: input.workspaceId,
     organizationId: input.organizationId,
     userId: input.userId,

apps/web/modules/ee/analysis/api/lib/cube-config.test.ts

@@ -35,7 +35,6 @@ describe("cube-config", () => {
 
     const { CUBE_API_TOKEN_TTL_SECONDS, CUBE_QUERY_SCOPE, getCubeApiConfig } = await import("./cube-config");
     const config = getCubeApiConfig({
-      tenantId: "workspace-1",
       workspaceId: "workspace-1",
       organizationId: "organization-1",
       userId: "user-1",
@@ -62,6 +61,27 @@ describe("cube-config", () => {
     expect(payload.exp! - payload.iat!).toBe(CUBE_API_TOKEN_TTL_SECONDS);
   });
 
+  test("derives the Cube tenant ID from workspaceId instead of trusting caller input", async () => {
+    setTestEnv();
+
+    const { getCubeApiConfig } = await import("./cube-config");
+    const config = getCubeApiConfig({
+      tenantId: "workspace-2",
+      workspaceId: "workspace-1",
+      organizationId: "organization-1",
+      userId: "user-1",
+      source: "charts.executeQueryAction",
+    } as unknown as Parameters<typeof getCubeApiConfig>[0]);
+
+    const payload = jwt.verify(config.token, "cube-secret", {
+      audience: "formbricks-cube-test",
+      issuer: "formbricks-web-test",
+    }) as jwt.JwtPayload;
+
+    expect(payload.tenantId).toBe("workspace-1");
+    expect(payload.workspaceId).toBe("workspace-1");
+  });
+
   test("preserves a full Cube API URL when it already contains /cubejs-api/v1", async () => {
     setTestEnv({
       CUBEJS_API_URL: "https://cube.formbricks.local/cubejs-api/v1/",

apps/web/modules/ee/analysis/api/lib/cube-config.ts

@@ -17,7 +17,6 @@ export type TCubeQuerySource =
   | "dashboards.widget";
 
 export type TCubeTenantScope = {
-  tenantId: string;
   workspaceId: string;
   organizationId: string;
   userId: string;
@@ -52,6 +51,8 @@ export const getCubeApiCredentials = () => {
   };
 };
 
+const deriveTenantIdFromWorkspace = (workspaceId: string): string => workspaceId;
+
 export const createCubeApiToken = (
   apiSecret: string,
   tenantScope: TCubeTenantScope,
@@ -64,11 +65,12 @@ export const createCubeApiToken = (
   } = {}
 ): TCubeApiToken => {
   const requestId = randomUUID();
+  const tenantId = deriveTenantIdFromWorkspace(tenantScope.workspaceId);
 
   return {
     token: jwt.sign(
       {
-        tenantId: tenantScope.tenantId,
+        tenantId,
         workspaceId: tenantScope.workspaceId,
         organizationId: tenantScope.organizationId,
         userId: tenantScope.userId,

apps/web/modules/ee/analysis/api/lib/cube-query-rewrite.test.ts

@@ -61,6 +61,15 @@ describe("cube queryRewrite", () => {
     ).toThrow(/invalid Cube query scope/);
   });
 
+  test("rejects tenantId and workspaceId claim mismatches", () => {
+    expect(() =>
+      queryRewrite(
+        { measures: ["FeedbackRecords.count"] },
+        { securityContext: { ...securityContext, tenantId: "workspace-2" } }
+      )
+    ).toThrow(/tenantId\/workspaceId mismatch/);
+  });
+
   test("rejects caller-supplied tenant filters", () => {
     expect(() =>
       queryRewrite(

apps/web/modules/ee/analysis/api/lib/cube-query.test.ts

@@ -85,6 +85,19 @@ describe("cube-query", () => {
         })
       ).toThrow(/Tenant filters are enforced by Cube/);
     });
+
+    test("rejects malformed member references without throwing runtime type errors", () => {
+      expect(() =>
+        validateCubeQueryMembers({
+          measures: ["FeedbackRecords.count", null],
+          dimensions: [{ member: "FeedbackRecords.sentiment" }],
+          segments: [0],
+          timeDimensions: [null, { dimension: null }],
+          filters: [null, { member: { name: "FeedbackRecords.sourceType" } }, { and: [0] }, { or: "bad" }],
+          order: [[null, "asc"], null],
+        } as unknown as TChartQuery)
+      ).toThrow(/Invalid query members.*invalid member reference/);
+    });
   });
 
   test("summarizes query members without raw filter values", () => {
@@ -108,4 +121,32 @@ describe("cube-query", () => {
     });
     expect(JSON.stringify(summary)).not.toContain("secret-value");
   });
+
+  test("summarizes only valid member names from malformed query shapes", () => {
+    const summary = getCubeQueryAuditSummary({
+      measures: ["FeedbackRecords.count", null],
+      dimensions: [{ member: "FeedbackRecords.sentiment" }],
+      timeDimensions: [null, { dimension: "FeedbackRecords.collectedAt" }],
+      filters: [
+        null,
+        { member: "FeedbackRecords.sourceType", operator: "equals", values: ["secret-value"] },
+        { and: [0, { member: "FeedbackRecords.sentiment", operator: "equals", values: ["positive"] }] },
+      ],
+      order: [
+        [null, "asc"],
+        ["FeedbackRecords.collectedAt", "desc"],
+      ],
+    } as unknown as TChartQuery);
+
+    expect(summary).toEqual({
+      measures: ["FeedbackRecords.count"],
+      dimensions: [],
+      segments: [],
+      timeDimensions: ["FeedbackRecords.collectedAt"],
+      filterMembers: ["FeedbackRecords.sentiment", "FeedbackRecords.sourceType"],
+      filterCount: 2,
+      orderMembers: ["FeedbackRecords.collectedAt"],
+    });
+    expect(JSON.stringify(summary)).not.toContain("secret-value");
+  });
 });

apps/web/modules/ee/analysis/api/lib/cube-query.ts

@@ -4,6 +4,7 @@ import type { TChartQuery } from "@formbricks/types/analysis";
 export const TENANT_MEMBER = "FeedbackRecords.tenantId";
 
 const ALLOWED_CUBE_PREFIXES = ["FeedbackRecords.", "TopicsUnnested."];
+const INVALID_MEMBER_REFERENCE = "invalid member reference";
 
 type TQueryAuditSummary = {
   measures: string[];
@@ -21,56 +22,134 @@ type TMemberValidationResult = {
   tenantMembers: string[];
 };
 
+type TCollectedMembers = {
+  members: string[];
+  invalidMemberCount: number;
+};
+
+type TCollectedFilterMembers = TCollectedMembers & {
+  count: number;
+};
+
 const uniqueSorted = (values: string[]): string[] =>
   Array.from(new Set(values)).sort((a, b) => a.localeCompare(b));
 
+const isRecord = (value: unknown): value is Record<string, unknown> =>
+  value !== null && typeof value === "object" && !Array.isArray(value);
+
+const collectStringMembers = (values: unknown): string[] => {
+  if (!Array.isArray(values)) {
+    return [];
+  }
+
+  return values.filter((value): value is string => typeof value === "string");
+};
+
+const collectTimeDimensionMembers = (timeDimensions: unknown): string[] => {
+  if (!Array.isArray(timeDimensions)) {
+    return [];
+  }
+
+  return timeDimensions
+    .map((timeDimension) => (isRecord(timeDimension) ? timeDimension.dimension : null))
+    .filter((dimension): dimension is string => typeof dimension === "string");
+};
+
 const validateMemberPrefix = (member: string): boolean =>
   ALLOWED_CUBE_PREFIXES.some((prefix) => member.startsWith(prefix));
 
-const collectOrderMembers = (order: TChartQuery["order"]): string[] => {
-  if (!order) {
-    return [];
+const collectOrderMembers = (order: unknown): TCollectedMembers => {
+  const members: string[] = [];
+  let invalidMemberCount = 0;
+
+  if (order === undefined || order === null) {
+    return { members, invalidMemberCount };
   }
 
   if (Array.isArray(order)) {
-    return order.map(([member]) => member).filter((member): member is string => typeof member === "string");
+    for (const orderEntry of order) {
+      if (Array.isArray(orderEntry) && typeof orderEntry[0] === "string") {
+        members.push(orderEntry[0]);
+      } else {
+        invalidMemberCount += 1;
+      }
+    }
+
+    return { members, invalidMemberCount };
   }
 
-  return Object.keys(order);
+  if (isRecord(order)) {
+    return { members: Object.keys(order), invalidMemberCount };
+  }
+
+  return { members, invalidMemberCount: 1 };
 };
 
 const collectFilterMembers = (
-  filters: TChartQuery["filters"],
+  filters: unknown,
   members: string[] = [],
-  count?: { value: number }
-): { members: string[]; count: number } => {
+  count?: { value: number },
+  invalidCount?: { value: number }
+): TCollectedFilterMembers => {
   const filterCount = count ?? { value: 0 };
+  const invalidMemberCount = invalidCount ?? { value: 0 };
+
+  if (filters === undefined) {
+    return { members, count: filterCount.value, invalidMemberCount: invalidMemberCount.value };
+  }
 
   if (!Array.isArray(filters)) {
-    return { members, count: filterCount.value };
+    invalidMemberCount.value += 1;
+    return { members, count: filterCount.value, invalidMemberCount: invalidMemberCount.value };
   }
 
   for (const filter of filters) {
-    const filterRecord = filter as { member?: unknown; dimension?: unknown };
-    const filterMembers = [
-      ...(typeof filterRecord.member === "string" ? [filterRecord.member] : []),
-      ...(typeof filterRecord.dimension === "string" ? [filterRecord.dimension] : []),
-    ];
-
-    for (const member of filterMembers) {
-      members.push(member);
-      filterCount.value += 1;
+    if (!isRecord(filter)) {
+      invalidMemberCount.value += 1;
+      continue;
     }
 
-    if ("and" in filter && Array.isArray(filter.and)) {
-      collectFilterMembers(filter.and, members, filterCount);
+    const hasMember = Object.hasOwn(filter, "member");
+    const hasDimension = Object.hasOwn(filter, "dimension");
+    const hasAnd = Object.hasOwn(filter, "and");
+    const hasOr = Object.hasOwn(filter, "or");
+
+    for (const member of [filter.member, filter.dimension]) {
+      if (typeof member === "string") {
+        members.push(member);
+        filterCount.value += 1;
+      }
     }
-    if ("or" in filter && Array.isArray(filter.or)) {
-      collectFilterMembers(filter.or, members, filterCount);
+
+    if (
+      (hasMember && typeof filter.member !== "string") ||
+      (hasDimension && typeof filter.dimension !== "string")
+    ) {
+      invalidMemberCount.value += 1;
+    }
+
+    if (hasAnd) {
+      if (Array.isArray(filter.and)) {
+        collectFilterMembers(filter.and, members, filterCount, invalidMemberCount);
+      } else {
+        invalidMemberCount.value += 1;
+      }
+    }
+
+    if (hasOr) {
+      if (Array.isArray(filter.or)) {
+        collectFilterMembers(filter.or, members, filterCount, invalidMemberCount);
+      } else {
+        invalidMemberCount.value += 1;
+      }
+    }
+
+    if (!hasMember && !hasDimension && !hasAnd && !hasOr) {
+      invalidMemberCount.value += 1;
     }
   }
 
-  return { members, count: filterCount.value };
+  return { members, count: filterCount.value, invalidMemberCount: invalidMemberCount.value };
 };
 
 const addValidatedMember = (member: string, result: TMemberValidationResult): void => {
@@ -81,6 +160,56 @@ const addValidatedMember = (member: string, result: TMemberValidationResult): vo
   }
 };
 
+const addInvalidMemberReferences = (result: TMemberValidationResult, count = 1): void => {
+  for (let index = 0; index < count; index += 1) {
+    result.invalidMembers.push(INVALID_MEMBER_REFERENCE);
+  }
+};
+
+const addValidatedMemberReference = (member: unknown, result: TMemberValidationResult): void => {
+  if (typeof member === "string") {
+    addValidatedMember(member, result);
+    return;
+  }
+
+  addInvalidMemberReferences(result);
+};
+
+const addValidatedMemberArray = (members: unknown, result: TMemberValidationResult): void => {
+  if (members === undefined) {
+    return;
+  }
+
+  if (!Array.isArray(members)) {
+    addInvalidMemberReferences(result);
+    return;
+  }
+
+  for (const member of members) {
+    addValidatedMemberReference(member, result);
+  }
+};
+
+const addValidatedTimeDimensions = (timeDimensions: unknown, result: TMemberValidationResult): void => {
+  if (timeDimensions === undefined) {
+    return;
+  }
+
+  if (!Array.isArray(timeDimensions)) {
+    addInvalidMemberReferences(result);
+    return;
+  }
+
+  for (const timeDimension of timeDimensions) {
+    if (isRecord(timeDimension) && typeof timeDimension.dimension === "string") {
+      addValidatedMember(timeDimension.dimension, result);
+      continue;
+    }
+
+    addInvalidMemberReferences(result);
+  }
+};
+
 /**
  * Validates all Cube member references controlled by users, saved charts, or AI output.
  * Tenant scoping is deliberately excluded from query JSON and enforced by Cube queryRewrite.
@@ -90,13 +219,21 @@ export const validateCubeQueryMembers = (query: TChartQuery): void => {
     invalidMembers: [],
     tenantMembers: [],
   };
+  const cubeQuery = isRecord(query) ? query : {};
+  if (!isRecord(query)) {
+    addInvalidMemberReferences(result);
+  }
 
-  for (const member of query.measures ?? []) addValidatedMember(member, result);
-  for (const member of query.dimensions ?? []) addValidatedMember(member, result);
-  for (const member of query.segments ?? []) addValidatedMember(member, result);
-  for (const timeDimension of query.timeDimensions ?? []) addValidatedMember(timeDimension.dimension, result);
-  for (const member of collectFilterMembers(query.filters).members) addValidatedMember(member, result);
-  for (const member of collectOrderMembers(query.order)) addValidatedMember(member, result);
+  const filters = collectFilterMembers(cubeQuery.filters);
+  const order = collectOrderMembers(cubeQuery.order);
+
+  addValidatedMemberArray(cubeQuery.measures, result);
+  addValidatedMemberArray(cubeQuery.dimensions, result);
+  addValidatedMemberArray(cubeQuery.segments, result);
+  addValidatedTimeDimensions(cubeQuery.timeDimensions, result);
+  for (const member of filters.members) addValidatedMember(member, result);
+  for (const member of order.members) addValidatedMember(member, result);
+  addInvalidMemberReferences(result, filters.invalidMemberCount + order.invalidMemberCount);
 
   if (result.tenantMembers.length > 0) {
     throw new Error(
@@ -116,16 +253,18 @@ export const validateCubeQueryMembers = (query: TChartQuery): void => {
 };
 
 export const getCubeQueryAuditSummary = (query: TChartQuery): TQueryAuditSummary => {
-  const filters = collectFilterMembers(query.filters);
+  const cubeQuery = isRecord(query) ? query : {};
+  const filters = collectFilterMembers(cubeQuery.filters);
+  const order = collectOrderMembers(cubeQuery.order);
 
   return {
-    measures: uniqueSorted(query.measures ?? []),
-    dimensions: uniqueSorted(query.dimensions ?? []),
-    segments: uniqueSorted(query.segments ?? []),
-    timeDimensions: uniqueSorted((query.timeDimensions ?? []).map(({ dimension }) => dimension)),
+    measures: uniqueSorted(collectStringMembers(cubeQuery.measures)),
+    dimensions: uniqueSorted(collectStringMembers(cubeQuery.dimensions)),
+    segments: uniqueSorted(collectStringMembers(cubeQuery.segments)),
+    timeDimensions: uniqueSorted(collectTimeDimensionMembers(cubeQuery.timeDimensions)),
     filterMembers: uniqueSorted(filters.members),
     filterCount: filters.count,
-    orderMembers: uniqueSorted(collectOrderMembers(query.order)),
-    ...(typeof query.limit === "number" ? { limit: query.limit } : {}),
+    orderMembers: uniqueSorted(order.members),
+    ...(typeof cubeQuery.limit === "number" ? { limit: cubeQuery.limit } : {}),
   };
 };

docker/cube/cube.js

@@ -79,16 +79,20 @@ function collectQueryMembers(query) {
 }
 
 function assertValidSecurityContext(securityContext) {
-  const tenantId = getRequiredStringClaim(securityContext, "tenantId");
+  const tenantIdClaim = getRequiredStringClaim(securityContext, "tenantId");
+  const workspaceId = getRequiredStringClaim(securityContext, "workspaceId");
   const scope = getRequiredStringClaim(securityContext, "scope");
 
   if (scope !== REQUIRED_SCOPE) {
     throw new Error("Cube query rejected: invalid Cube query scope");
   }
+  if (tenantIdClaim !== workspaceId) {
+    throw new Error("Cube query rejected: tenantId/workspaceId mismatch");
+  }
 
   return {
-    tenantId,
-    workspaceId: getRequiredStringClaim(securityContext, "workspaceId"),
+    tenantId: workspaceId,
+    workspaceId,
     organizationId: getRequiredStringClaim(securityContext, "organizationId"),
     userId: getRequiredStringClaim(securityContext, "userId"),
     requestId: getRequiredStringClaim(securityContext, "jti"),
@@ -134,7 +138,7 @@ function queryRewrite(query, rewriteContext) {
       {
         member: TENANT_MEMBER,
         operator: "equals",
-        values: [context.tenantId],
+        values: [context.workspaceId],
       },
     ],
   };

docs/development/technical-handbook/cube-tenant-isolation.mdx

@@ -20,14 +20,26 @@ The controls assume query bodies are attacker-influenced. Tenant identity is nev
 
 ## Enforcement Flow
 
-1. Server actions and server components authorize workspace access in the Next.js app.
-2. The app validates the Cube query and rejects any `FeedbackRecords.tenantId` member supplied by users, saved
-   charts, or AI output, including filters, dimensions, time dimensions, and order clauses.
-3. The app mints a short-lived JWT per Cube request with `tenantId`, `workspaceId`, `organizationId`, `userId`,
-   `scope`, `iss`, `aud`, `jti`, and `exp` claims.
-4. Cube verifies the JWT and exposes the claims through `securityContext`.
-5. Cube `queryRewrite` rejects missing tenant context, rejects caller-supplied tenant member usage, and appends
-   `FeedbackRecords.tenantId = securityContext.tenantId` to every query.
+<Steps>
+  <Step title="Authorize workspace access">
+    Server actions and server components authorize workspace access in the Next.js app.
+  </Step>
+  <Step title="Validate the Cube query">
+    The app validates the Cube query and rejects any `FeedbackRecords.tenantId` member supplied by users,
+    saved charts, or AI output, including filters, dimensions, time dimensions, and order clauses.
+  </Step>
+  <Step title="Mint a short-lived JWT">
+    The app mints a short-lived JWT per Cube request with `tenantId`, `workspaceId`, `organizationId`,
+    `userId`, `scope`, `iss`, `aud`, `jti`, and `exp` claims.
+  </Step>
+  <Step title="Verify the JWT in Cube">
+    Cube verifies the JWT and exposes the claims through `securityContext`.
+  </Step>
+  <Step title="Enforce the tenant filter">
+    Cube `queryRewrite` rejects missing tenant context, rejects caller-supplied tenant member usage, and
+    appends `FeedbackRecords.tenantId = securityContext.tenantId` to every query.
+  </Step>
+</Steps>
 
 ## Audit Evidence