trace api to clickhouse

agents-api/src/__tests__/utils/signozHelpers.test.ts

@@ -0,0 +1,121 @@
+import { describe, expect, it } from 'vitest';
+import { enforceQuerySecurity } from '../../utils/signozHelpers';
+
+describe('enforceQuerySecurity', () => {
+  const validQuery = "SELECT * FROM t WHERE attributes_string['tenant.id'] = {{.tenant_id}}";
+
+  it('injects tenant_id variable', () => {
+    const payload = {
+      compositeQuery: { chQueries: { A: { query: validQuery } } },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1');
+    expect(result.error).toBeUndefined();
+    expect(result.payload.variables.tenant_id).toBe('tenant-1');
+  });
+
+  it('injects project_id variable when provided', () => {
+    const payload = {
+      compositeQuery: { chQueries: { A: { query: validQuery } } },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1', 'project-1');
+    expect(result.payload.variables).toEqual({
+      tenant_id: 'tenant-1',
+      project_id: 'project-1',
+    });
+  });
+
+  it('does not inject project_id when not provided', () => {
+    const payload = {
+      compositeQuery: { chQueries: { A: { query: validQuery } } },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1');
+    expect(result.payload.variables.project_id).toBeUndefined();
+  });
+
+  it('overwrites client-provided tenant_id (anti-spoofing)', () => {
+    const payload = {
+      variables: { tenant_id: 'attacker-tenant' },
+      compositeQuery: { chQueries: { A: { query: validQuery } } },
+    };
+    const result = enforceQuerySecurity(payload, 'legitimate-tenant');
+    expect(result.payload.variables.tenant_id).toBe('legitimate-tenant');
+  });
+
+  it('overwrites client-provided project_id (anti-spoofing)', () => {
+    const payload = {
+      variables: { project_id: 'attacker-project' },
+      compositeQuery: { chQueries: { A: { query: validQuery } } },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1', 'real-project');
+    expect(result.payload.variables.project_id).toBe('real-project');
+  });
+
+  it('initializes variables object when missing', () => {
+    const payload = {
+      compositeQuery: { chQueries: { A: { query: validQuery } } },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1', 'project-1');
+    expect(result.payload.variables).toEqual({
+      tenant_id: 'tenant-1',
+      project_id: 'project-1',
+    });
+  });
+
+  it('rejects query missing {{.tenant_id}} reference', () => {
+    const payload = {
+      compositeQuery: {
+        chQueries: {
+          malicious: {
+            query: 'SELECT * FROM signoz_traces.distributed_signoz_index_v3 LIMIT 1000',
+          },
+        },
+      },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1');
+    expect(result.error).toBe('Query "malicious" is missing required {{.tenant_id}} tenant filter');
+  });
+
+  it('rejects when any one of multiple queries is missing tenant filter', () => {
+    const payload = {
+      compositeQuery: {
+        chQueries: {
+          good: { query: validQuery },
+          bad: { query: 'SELECT count() FROM t' },
+        },
+      },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1');
+    expect(result.error).toContain('bad');
+  });
+
+  it('passes when all queries reference {{.tenant_id}}', () => {
+    const payload = {
+      compositeQuery: {
+        chQueries: {
+          q1: { query: validQuery },
+          q2: {
+            query: `SELECT count() FROM t WHERE attributes_string['tenant.id'] = {{.tenant_id}}`,
+          },
+        },
+      },
+    };
+    const result = enforceQuerySecurity(payload, 'tenant-1');
+    expect(result.error).toBeUndefined();
+  });
+
+  it('passes when no chQueries present (non-clickhouse payload)', () => {
+    const payload = { compositeQuery: {} };
+    const result = enforceQuerySecurity(payload, 'tenant-1');
+    expect(result.error).toBeUndefined();
+    expect(result.payload.variables.tenant_id).toBe('tenant-1');
+  });
+
+  it('does not mutate the original payload', () => {
+    const original = {
+      variables: { tenant_id: 'original' },
+      compositeQuery: { chQueries: { A: { query: validQuery } } },
+    };
+    enforceQuerySecurity(original, 'new-tenant');
+    expect(original.variables.tenant_id).toBe('original');
+  });
+});

agents-api/src/domains/manage/routes/signoz.ts

@@ -4,7 +4,7 @@ import { Hono } from 'hono';
 import { env } from '../../../env';
 import { getLogger } from '../../../logger';
 import type { ManageAppVariables } from '../../../types/app';
-import { enforceSecurityFilters } from '../../../utils/signozHelpers';
+import { enforceQuerySecurity } from '../../../utils/signozHelpers';
 
 const logger = getLogger('signoz-proxy');
 
@@ -61,8 +61,12 @@ app.post('/query', async (c) => {
     }
   }
 
-  // Always enforce server-side tenant filter, and project filter if provided
-  payload = enforceSecurityFilters(payload, tenantId, requestedProjectId);
+  const secured = enforceQuerySecurity(payload, tenantId, requestedProjectId);
+  if (secured.error) {
+    logger.warn({ tenantId, error: secured.error }, 'Query rejected: missing tenant filter');
+    return c.json({ error: 'Bad Request', message: secured.error }, 400);
+  }
+  payload = secured.payload;
   logger.debug({ tenantId, projectId: requestedProjectId }, 'Security filters enforced');
 
   const signozUrl = env.SIGNOZ_URL || env.PUBLIC_SIGNOZ_URL;
@@ -201,19 +205,21 @@ app.post('/query-batch', async (c) => {
     'SIGNOZ-API-KEY': signozApiKey,
   };
 
-  try {
-    // Step 1: Execute pagination query
-    const securedPagination = enforceSecurityFilters(
-      paginationPayload,
-      tenantId,
-      requestedProjectId
+  const securedPagination = enforceQuerySecurity(paginationPayload, tenantId, requestedProjectId);
+  if (securedPagination.error) {
+    logger.warn(
+      { tenantId, error: securedPagination.error },
+      'Pagination query rejected: missing tenant filter'
     );
-    const step1 = await axios.post(signozEndpoint, securedPagination, {
+    return c.json({ error: 'Bad Request', message: securedPagination.error }, 400);
+  }
+
+  try {
+    const step1 = await axios.post(signozEndpoint, securedPagination.payload, {
       headers: signozHeaders,
       timeout: 30000,
     });
 
-    // Extract conversation IDs from the pageConversations result
     const pageResult = step1.data?.data?.result?.find(
       (r: any) => r?.queryName === 'pageConversations'
     );
@@ -225,10 +231,16 @@ app.post('/query-batch', async (c) => {
       return c.json({ paginationResponse: step1.data, detailResponse: null });
     }
 
-    // Step 2: Inject conversation IDs into the detail template and execute
     const detailWithIds = injectConversationIdFilter(detailPayloadTemplate, conversationIds);
-    const securedDetail = enforceSecurityFilters(detailWithIds, tenantId, requestedProjectId);
-    const step2 = await axios.post(signozEndpoint, securedDetail, {
+    const securedDetail = enforceQuerySecurity(detailWithIds, tenantId, requestedProjectId);
+    if (securedDetail.error) {
+      logger.warn(
+        { tenantId, error: securedDetail.error },
+        'Detail query rejected: missing tenant filter'
+      );
+      return c.json({ error: 'Bad Request', message: securedDetail.error }, 400);
+    }
+    const step2 = await axios.post(signozEndpoint, securedDetail.payload, {
       headers: signozHeaders,
       timeout: 30000,
     });
@@ -261,39 +273,21 @@ app.post('/query-batch', async (c) => {
 });
 
 /**
- * Inject a `conversation.id IN [...]` filter into every builder query
- * of a SigNoz composite query payload.
+ * Inject conversation IDs into the __CONVERSATION_IDS__ placeholder
+ * in ClickHouse SQL queries within a SigNoz composite query payload.
  */
 function injectConversationIdFilter(payload: any, conversationIds: string[]): any {
   const modified = JSON.parse(JSON.stringify(payload));
-  const builderQueries = modified.compositeQuery?.builderQueries;
-  if (!builderQueries) return modified;
-
-  const inFilter = {
-    key: {
-      key: 'conversation.id',
-      dataType: 'string',
-      type: 'tag',
-      isColumn: false,
-      isJSON: false,
-      id: 'false',
-    },
-    op: 'in',
-    value: conversationIds,
-  };
+  const chQueries = modified.compositeQuery?.chQueries;
+  if (!chQueries) return modified;
+
+  const inClause = conversationIds.map((id: string) => `'${id.replace(/'/g, "''")}'`).join(',');
 
-  for (const queryKey in builderQueries) {
-    const query = builderQueries[queryKey];
-    if (!query.filters) {
-      query.filters = { op: 'AND', items: [] };
+  for (const key of Object.keys(chQueries)) {
+    if (chQueries[key]?.query) {
+      chQueries[key].query = chQueries[key].query.replaceAll('__CONVERSATION_IDS__', inClause);
     }
-    // Remove any existing conversation.id IN filter to avoid duplication
-    query.filters.items = query.filters.items.filter(
-      (item: any) => !(item.key?.key === 'conversation.id' && item.op === 'in')
-    );
-    query.filters.items.push(inFilter);
   }
-
   return modified;
 }
 

agents-api/src/utils/signozHelpers.ts

@@ -1,55 +1,36 @@
 /**
- * Helper function to enforce projectId filter on SigNoz queries.
- * This modifies the query payload to ensure all builder queries include
- * a server-side project.id filter, preventing client-side filter bypass.
+ * Enforce tenant/project security on ClickHouse SQL queries by:
+ * 1. Injecting server-side variables (tenant_id, project_id) — always overrides client values
+ * 2. Validating every chQuery references {{.tenant_id}} to prevent tenant isolation bypass
+ *
+ * Returns null if valid, or an error message string if a query is missing the tenant filter.
  */
-export function enforceSecurityFilters(payload: any, tenantId: string, projectId?: string): any {
+export function enforceQuerySecurity(
+  payload: any,
+  tenantId: string,
+  projectId?: string
+): { payload: any; error?: string } {
   const modifiedPayload = JSON.parse(JSON.stringify(payload));
+  if (!modifiedPayload.variables) {
+    modifiedPayload.variables = {};
+  }
+  modifiedPayload.variables.tenant_id = tenantId;
+  if (projectId) {
+    modifiedPayload.variables.project_id = projectId;
+  }
 
-  if (modifiedPayload.compositeQuery?.builderQueries) {
-    for (const queryKey in modifiedPayload.compositeQuery.builderQueries) {
-      const query = modifiedPayload.compositeQuery.builderQueries[queryKey];
-
-      if (!query.filters) {
-        query.filters = { op: 'AND', items: [] };
-      }
-
-      // Remove any existing tenant.id and project.id filters to prevent bypass
-      query.filters.items = query.filters.items.filter(
-        (item: any) => item.key?.key !== 'tenant.id' && item.key?.key !== 'project.id'
-      );
-
-      // Always add server-side tenant filter
-      query.filters.items.push({
-        key: {
-          key: 'tenant.id',
-          dataType: 'string',
-          type: 'tag',
-          isColumn: false,
-          isJSON: false,
-          id: 'false',
-        },
-        op: '=',
-        value: tenantId,
-      });
-
-      // Add server-side project filter if provided
-      if (projectId) {
-        query.filters.items.push({
-          key: {
-            key: 'project.id',
-            dataType: 'string',
-            type: 'tag',
-            isColumn: false,
-            isJSON: false,
-            id: 'false',
-          },
-          op: '=',
-          value: projectId,
-        });
+  const chQueries = modifiedPayload.compositeQuery?.chQueries;
+  if (chQueries) {
+    for (const [name, entry] of Object.entries(chQueries)) {
+      const query = (entry as any)?.query;
+      if (typeof query === 'string' && !query.includes('{{.tenant_id}}')) {
+        return {
+          payload: modifiedPayload,
+          error: `Query "${name}" is missing required {{.tenant_id}} tenant filter`,
+        };
       }
     }
   }
 
-  return modifiedPayload;
+  return { payload: modifiedPayload };
 }