diff --git a/internal-packages/clickhouse/src/client/tsql.ts b/internal-packages/clickhouse/src/client/tsql.ts index ec7a3a3f8..9a2328f46 100644 --- a/internal-packages/clickhouse/src/client/tsql.ts +++ b/internal-packages/clickhouse/src/client/tsql.ts @@ -29,12 +29,12 @@ export interface ExecuteTSQLOptions { query: string; /** The Zod schema for validating output rows */ schema: TOut; - /** The organization ID for tenant isolation */ + /** The organization ID for tenant isolation (required) */ organizationId: string; - /** The project ID for tenant isolation */ - projectId: string; - /** The environment ID for tenant isolation */ - environmentId: string; + /** The project ID for tenant isolation (optional - omit to query across all projects) */ + projectId?: string; + /** The environment ID for tenant isolation (optional - omit to query across all environments) */ + environmentId?: string; /** Schema registry defining allowed tables and columns */ tableSchema: TableSchema[]; /** Optional ClickHouse query settings */ diff --git a/internal-packages/clickhouse/src/tsql.test.ts b/internal-packages/clickhouse/src/tsql.test.ts index 9bc917d21..b463828c2 100644 --- a/internal-packages/clickhouse/src/tsql.test.ts +++ b/internal-packages/clickhouse/src/tsql.test.ts @@ -472,3 +472,286 @@ describe("TSQL Integration Tests", () => { expect(rows?.every((r) => r.task_identifier.startsWith("email"))).toBe(true); }); }); + +describe("TSQL Optional Tenant Filter Tests", () => { + clickhouseTest( + "should query across all projects when projectId is omitted", + async ({ clickhouseContainer }) => { + const client = new ClickhouseClient({ + name: "test", + url: clickhouseContainer.getConnectionUrl(), + }); + + const insert = insertTaskRuns(client, { async_insert: 0 }); + + // Insert data across multiple projects in the same org + await insert([ + createTaskRun({ + run_id: "run_proj1_a", + organization_id: "org_multi", + project_id: "proj_1", + environment_id: "env_dev", + }), + createTaskRun({ + run_id: "run_proj1_b", + organization_id: "org_multi", + project_id: "proj_1", + environment_id: "env_dev", + }), + createTaskRun({ + run_id: "run_proj2_a", + organization_id: "org_multi", + project_id: "proj_2", + environment_id: "env_dev", + }), + createTaskRun({ + run_id: "run_proj3_a", + organization_id: "org_multi", + project_id: "proj_3", + environment_id: "env_prod", + }), + // Different org - should not be returned + createTaskRun({ + run_id: "run_other_org", + organization_id: "org_other", + project_id: "proj_other", + environment_id: "env_other", + }), + ]); + + // Query across all projects (omit projectId and environmentId) + const [error, rows] = await executeTSQL(client, { + name: "test-cross-project-query", + query: "SELECT run_id FROM task_runs", + schema: z.object({ run_id: z.string() }), + organizationId: "org_multi", + // projectId and environmentId omitted - query across all + tableSchema: [taskRunsSchema], + }); + + expect(error).toBeNull(); + expect(rows).toHaveLength(4); // All runs from org_multi + expect(rows?.map((r) => r.run_id).sort()).toEqual([ + "run_proj1_a", + "run_proj1_b", + "run_proj2_a", + "run_proj3_a", + ]); + } + ); + + clickhouseTest( + "should query across all environments in a project when environmentId is omitted", + async ({ clickhouseContainer }) => { + const client = new ClickhouseClient({ + name: "test", + url: clickhouseContainer.getConnectionUrl(), + }); + + const insert = insertTaskRuns(client, { async_insert: 0 }); + + // Insert data across multiple environments in the same project + await insert([ + createTaskRun({ + run_id: "run_dev_1", + organization_id: "org_envtest", + project_id: "proj_envtest", + environment_id: "env_development", + }), + createTaskRun({ + run_id: "run_staging_1", + organization_id: "org_envtest", + project_id: "proj_envtest", + environment_id: "env_staging", + }), + createTaskRun({ + run_id: "run_prod_1", + organization_id: "org_envtest", + project_id: "proj_envtest", + environment_id: "env_production", + }), + // Different project - should not be returned + createTaskRun({ + run_id: "run_other_proj", + organization_id: "org_envtest", + project_id: "proj_other", + environment_id: "env_development", + }), + ]); + + // Query across all environments (omit environmentId only) + const [error, rows] = await executeTSQL(client, { + name: "test-cross-env-query", + query: "SELECT run_id FROM task_runs", + schema: z.object({ run_id: z.string() }), + organizationId: "org_envtest", + projectId: "proj_envtest", + // environmentId omitted - query across all environments + tableSchema: [taskRunsSchema], + }); + + expect(error).toBeNull(); + expect(rows).toHaveLength(3); // All runs from proj_envtest across all envs + expect(rows?.map((r) => r.run_id).sort()).toEqual([ + "run_dev_1", + "run_prod_1", + "run_staging_1", + ]); + } + ); + + clickhouseTest( + "should still enforce org isolation when querying across projects", + async ({ clickhouseContainer }) => { + const client = new ClickhouseClient({ + name: "test", + url: clickhouseContainer.getConnectionUrl(), + }); + + const insert = insertTaskRuns(client, { async_insert: 0 }); + + // Insert data for multiple orgs + await insert([ + createTaskRun({ + run_id: "run_org1_a", + organization_id: "org_isolation_1", + project_id: "proj_a", + environment_id: "env_a", + }), + createTaskRun({ + run_id: "run_org1_b", + organization_id: "org_isolation_1", + project_id: "proj_b", + environment_id: "env_b", + }), + createTaskRun({ + run_id: "run_org2_a", + organization_id: "org_isolation_2", + project_id: "proj_c", + environment_id: "env_c", + }), + createTaskRun({ + run_id: "run_org2_b", + organization_id: "org_isolation_2", + project_id: "proj_d", + environment_id: "env_d", + }), + ]); + + // Query org1 across all projects - should NOT see org2's data + const [error1, rows1] = await executeTSQL(client, { + name: "test-org-isolation-1", + query: "SELECT run_id FROM task_runs", + schema: z.object({ run_id: z.string() }), + organizationId: "org_isolation_1", + // projectId and environmentId omitted + tableSchema: [taskRunsSchema], + }); + + expect(error1).toBeNull(); + expect(rows1).toHaveLength(2); + expect(rows1?.map((r) => r.run_id).sort()).toEqual(["run_org1_a", "run_org1_b"]); + + // Query org2 across all projects - should NOT see org1's data + const [error2, rows2] = await executeTSQL(client, { + name: "test-org-isolation-2", + query: "SELECT run_id FROM task_runs", + schema: z.object({ run_id: z.string() }), + organizationId: "org_isolation_2", + // projectId and environmentId omitted + tableSchema: [taskRunsSchema], + }); + + expect(error2).toBeNull(); + expect(rows2).toHaveLength(2); + expect(rows2?.map((r) => r.run_id).sort()).toEqual(["run_org2_a", "run_org2_b"]); + } + ); + + clickhouseTest( + "should prevent OR clause bypass with org-only filter", + async ({ clickhouseContainer }) => { + const client = new ClickhouseClient({ + name: "test", + url: clickhouseContainer.getConnectionUrl(), + }); + + const insert = insertTaskRuns(client, { async_insert: 0 }); + + await insert([ + createTaskRun({ + run_id: "run_victim", + organization_id: "org_victim", + project_id: "proj_victim", + environment_id: "env_victim", + status: "SECRET", + }), + createTaskRun({ + run_id: "run_attacker", + organization_id: "org_attacker", + project_id: "proj_attacker", + environment_id: "env_attacker", + status: "PUBLIC", + }), + ]); + + // Attacker tries to use OR 1=1 to bypass org filter + const [error, rows] = await executeTSQL(client, { + name: "test-or-bypass-attempt", + query: "SELECT run_id, status FROM task_runs WHERE status = 'COMPLETED' OR 1=1", + schema: z.object({ run_id: z.string(), status: z.string() }), + organizationId: "org_attacker", + // No project/env filter - but org filter should still protect + tableSchema: [taskRunsSchema], + }); + + expect(error).toBeNull(); + // Should only get attacker's data, not victim's + expect(rows).toHaveLength(1); + expect(rows?.[0].run_id).toBe("run_attacker"); + expect(rows?.find((r) => r.run_id === "run_victim")).toBeUndefined(); + } + ); + + clickhouseTest( + "should work with createTSQLExecutor and optional filters", + async ({ clickhouseContainer }) => { + const client = new ClickhouseClient({ + name: "test", + url: clickhouseContainer.getConnectionUrl(), + }); + + const insert = insertTaskRuns(client, { async_insert: 0 }); + + await insert([ + createTaskRun({ + run_id: "run_exec_1", + organization_id: "org_executor_test", + project_id: "proj_1", + environment_id: "env_1", + }), + createTaskRun({ + run_id: "run_exec_2", + organization_id: "org_executor_test", + project_id: "proj_2", + environment_id: "env_2", + }), + ]); + + const tsql = createTSQLExecutor(client, [taskRunsSchema]); + + // Use executor with org-only filter + const [error, rows] = await tsql.execute({ + name: "test-executor-optional", + query: "SELECT run_id FROM task_runs", + schema: z.object({ run_id: z.string() }), + organizationId: "org_executor_test", + // projectId and environmentId omitted + }); + + expect(error).toBeNull(); + expect(rows).toHaveLength(2); + expect(rows?.map((r) => r.run_id).sort()).toEqual(["run_exec_1", "run_exec_2"]); + } + ); +}); diff --git a/internal-packages/tsql/src/index.ts b/internal-packages/tsql/src/index.ts index 4dbca6a1c..c669ca6a0 100644 --- a/internal-packages/tsql/src/index.ts +++ b/internal-packages/tsql/src/index.ts @@ -192,12 +192,12 @@ export function parseTSQLExpr(expr: string): Expression { * Options for compiling a TSQL query to ClickHouse SQL */ export interface CompileTSQLOptions { - /** The organization ID for tenant isolation */ + /** The organization ID for tenant isolation (required) */ organizationId: string; - /** The project ID for tenant isolation */ - projectId: string; - /** The environment ID for tenant isolation */ - environmentId: string; + /** The project ID for tenant isolation (optional - omit to query across all projects) */ + projectId?: string; + /** The environment ID for tenant isolation (optional - omit to query across all environments) */ + environmentId?: string; /** Schema definitions for allowed tables and columns */ tableSchema: TableSchema[]; /** Optional query settings */ diff --git a/internal-packages/tsql/src/query/printer.ts b/internal-packages/tsql/src/query/printer.ts index 3cbbd9ccf..b58426583 100644 --- a/internal-packages/tsql/src/query/printer.ts +++ b/internal-packages/tsql/src/query/printer.ts @@ -519,12 +519,14 @@ export class ClickHousePrinter { * Create a WHERE clause expression for tenant isolation * Note: We use just the column name without table prefix since ClickHouse * requires the actual table name (task_runs_v2), not the TSQL alias (task_runs) + * + * Organization ID is always required. Project ID and Environment ID are optional - + * if not provided, the query will return results across all projects/environments. */ - private createTenantGuard(tableSchema: TableSchema, _tableAlias: string): And { + private createTenantGuard(tableSchema: TableSchema, _tableAlias: string): And | CompareOperation { const { tenantColumns } = tableSchema; - // Create equality comparisons for each tenant column - // Use just the column name - ClickHouse will resolve it correctly + // Organization guard is always required const orgGuard: CompareOperation = { expression_type: "compare_operation", op: CompareOperationOp.Eq, @@ -532,23 +534,39 @@ export class ClickHousePrinter { right: { expression_type: "constant", value: this.context.organizationId } as Constant, }; - const projectGuard: CompareOperation = { - expression_type: "compare_operation", - op: CompareOperationOp.Eq, - left: { expression_type: "field", chain: [tenantColumns.projectId] } as Field, - right: { expression_type: "constant", value: this.context.projectId } as Constant, - }; + // Collect all guards - org is always included + const guards: CompareOperation[] = [orgGuard]; - const envGuard: CompareOperation = { - expression_type: "compare_operation", - op: CompareOperationOp.Eq, - left: { expression_type: "field", chain: [tenantColumns.environmentId] } as Field, - right: { expression_type: "constant", value: this.context.environmentId } as Constant, - }; + // Only add project guard if projectId is provided + if (this.context.projectId !== undefined) { + const projectGuard: CompareOperation = { + expression_type: "compare_operation", + op: CompareOperationOp.Eq, + left: { expression_type: "field", chain: [tenantColumns.projectId] } as Field, + right: { expression_type: "constant", value: this.context.projectId } as Constant, + }; + guards.push(projectGuard); + } + + // Only add environment guard if environmentId is provided + if (this.context.environmentId !== undefined) { + const envGuard: CompareOperation = { + expression_type: "compare_operation", + op: CompareOperationOp.Eq, + left: { expression_type: "field", chain: [tenantColumns.environmentId] } as Field, + right: { expression_type: "constant", value: this.context.environmentId } as Constant, + }; + guards.push(envGuard); + } + + // If only org guard, return it directly (no need for AND wrapper) + if (guards.length === 1) { + return orgGuard; + } return { expression_type: "and", - exprs: [orgGuard, projectGuard, envGuard], + exprs: guards, }; } diff --git a/internal-packages/tsql/src/query/printer_context.ts b/internal-packages/tsql/src/query/printer_context.ts index 02aec6dd3..5b0da55fc 100644 --- a/internal-packages/tsql/src/query/printer_context.ts +++ b/internal-packages/tsql/src/query/printer_context.ts @@ -61,12 +61,12 @@ export class PrinterContext { readonly errors: QueryNotice[] = []; constructor( - /** The organization ID for tenant isolation */ + /** The organization ID for tenant isolation (required) */ public readonly organizationId: string, - /** The project ID for tenant isolation */ - public readonly projectId: string, - /** The environment ID for tenant isolation */ - public readonly environmentId: string, + /** The project ID for tenant isolation (optional - omit to query across all projects) */ + public readonly projectId: string | undefined, + /** The environment ID for tenant isolation (optional - omit to query across all environments) */ + public readonly environmentId: string | undefined, /** Schema registry containing allowed tables and columns */ public readonly schema: SchemaRegistry, /** Query execution settings */ @@ -177,9 +177,12 @@ export class PrinterContext { * Options for creating a printer context */ export interface PrinterContextOptions { + /** The organization ID for tenant isolation (required) */ organizationId: string; - projectId: string; - environmentId: string; + /** The project ID for tenant isolation (optional - omit to query across all projects) */ + projectId?: string; + /** The environment ID for tenant isolation (optional - omit to query across all environments) */ + environmentId?: string; schema: SchemaRegistry; settings?: QuerySettings; } diff --git a/internal-packages/tsql/src/query/security.test.ts b/internal-packages/tsql/src/query/security.test.ts index ee294c35e..e764c7495 100644 --- a/internal-packages/tsql/src/query/security.test.ts +++ b/internal-packages/tsql/src/query/security.test.ts @@ -398,6 +398,158 @@ describe("Parameter Safety", () => { }); }); +describe("Optional Tenant Filters", () => { + describe("Organization ID is always required", () => { + it("should always inject organization guard even with optional project/env", () => { + const { sql, params } = compile("SELECT * FROM task_runs", { + projectId: undefined, + environmentId: undefined, + }); + + // Must contain organization_id + expect(sql).toContain("organization_id"); + expect(Object.values(params)).toContain("org_tenant1"); + + // Should NOT contain project_id or environment_id guards + expect(sql).not.toContain("project_id"); + expect(sql).not.toContain("environment_id"); + }); + }); + + describe("Project ID is optional", () => { + it("should inject org and project guards when project is provided", () => { + const { sql, params } = compile("SELECT * FROM task_runs", { + projectId: "proj_tenant1", + environmentId: undefined, + }); + + // Must contain organization_id and project_id + expect(sql).toContain("organization_id"); + expect(sql).toContain("project_id"); + expect(Object.values(params)).toContain("org_tenant1"); + expect(Object.values(params)).toContain("proj_tenant1"); + + // Should NOT contain environment_id guard + expect(sql).not.toContain("environment_id"); + }); + + it("should allow querying across all projects when projectId is omitted", () => { + const { sql, params } = compile("SELECT * FROM task_runs", { + projectId: undefined, + environmentId: undefined, + }); + + // Only org guard should be present + expect(sql).toContain("organization_id"); + expect(Object.values(params)).toContain("org_tenant1"); + expect(sql).not.toContain("project_id"); + }); + }); + + describe("Environment ID is optional", () => { + it("should inject org, project, and env guards when all provided", () => { + const { sql, params } = compile("SELECT * FROM task_runs"); + + // All three should be present (default options include all) + expect(sql).toContain("organization_id"); + expect(sql).toContain("project_id"); + expect(sql).toContain("environment_id"); + expect(Object.values(params)).toContain("org_tenant1"); + expect(Object.values(params)).toContain("proj_tenant1"); + expect(Object.values(params)).toContain("env_tenant1"); + }); + + it("should allow querying across all environments when environmentId is omitted", () => { + const { sql, params } = compile("SELECT * FROM task_runs", { + projectId: "proj_tenant1", + environmentId: undefined, + }); + + // Org and project guards should be present + expect(sql).toContain("organization_id"); + expect(sql).toContain("project_id"); + expect(Object.values(params)).toContain("org_tenant1"); + expect(Object.values(params)).toContain("proj_tenant1"); + + // Environment guard should NOT be present + expect(sql).not.toContain("environment_id"); + }); + }); + + describe("Cross-tenant security with optional filters", () => { + it("should still prevent cross-org access with org-only filter", () => { + const { sql, params } = compile( + "SELECT * FROM task_runs WHERE organization_id = 'org_other'", + { + projectId: undefined, + environmentId: undefined, + } + ); + + // Our org guard should still be enforced + expect(Object.values(params)).toContain("org_tenant1"); + }); + + it("should apply org guard to all tables in JOIN when using org-only filter", () => { + const { sql } = compile( + ` + SELECT r.id, e.event_type + FROM task_runs r + JOIN task_events e ON r.id = e.run_id + `, + { + projectId: undefined, + environmentId: undefined, + } + ); + + // Both tables should have org guards + const orgIdMatches = sql.match(/organization_id/g) || []; + expect(orgIdMatches.length).toBeGreaterThanOrEqual(2); + + // Project and environment should NOT appear + expect(sql).not.toContain("project_id"); + expect(sql).not.toContain("environment_id"); + }); + + it("should apply org guard to UNION queries when using org-only filter", () => { + const { sql } = compile( + ` + SELECT id, status FROM task_runs WHERE status = 'completed' + UNION ALL + SELECT id, status FROM task_runs WHERE status = 'failed' + `, + { + projectId: undefined, + environmentId: undefined, + } + ); + + // Both parts should have org guards + const orgIdMatches = sql.match(/organization_id/g) || []; + expect(orgIdMatches.length).toBeGreaterThanOrEqual(2); + }); + + it("should apply org guard to subqueries when using org-only filter", () => { + const { sql, params } = compile( + ` + SELECT * FROM task_runs + WHERE id IN (SELECT run_id FROM task_events) + `, + { + projectId: undefined, + environmentId: undefined, + } + ); + + // Both main query and subquery should have org guards + expect(Object.values(params)).toContain("org_tenant1"); + const orgIdMatches = sql.match(/organization_id/g) || []; + expect(orgIdMatches.length).toBeGreaterThanOrEqual(2); + }); + }); +}); + describe("Edge Cases", () => { it("should handle empty string values", () => { const { params } = compile("SELECT * FROM task_runs WHERE status = ''");