From 59b6f126ee38cb1b340caff771a19f2a02170ad7 Mon Sep 17 00:00:00 2001 From: Superlog app Date: Sun, 14 Jun 2026 00:41:29 +0000 Subject: [PATCH] [superlog] Skip unsupported filter fields per query type instead of throwing --- packages/ai/src/query/simple-builder.test.ts | 38 +++++++++++++++----- packages/ai/src/query/simple-builder.ts | 15 ++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/ai/src/query/simple-builder.test.ts b/packages/ai/src/query/simple-builder.test.ts index d8191378f..c7dc4f03b 100644 --- a/packages/ai/src/query/simple-builder.test.ts +++ b/packages/ai/src/query/simple-builder.test.ts @@ -346,23 +346,28 @@ describe("SimpleQueryBuilder.compile", () => { expect(params.f0).toBe("mobile"); }); - it("throws on disallowed filter field", () => { + it("silently skips disallowed filter fields rather than throwing", () => { const filters: Filter[] = [ { field: "secret_col", op: "eq", value: "x" }, ]; - expect(() => - compile({ allowedFilters: ["country"] }, { filters }) - ).toThrow("not permitted"); + // Unsupported filter fields are skipped so that a multi-query batch + // (e.g. an "href" filter valid for outbound_links but not for "country") + // does not break unrelated queries. + const { sql } = compile({ allowedFilters: ["country"] }, { filters }); + expect(sql).not.toContain("secret_col"); }); - it("rejects unknown filter fields when allowedFilters is not configured", () => { + it("silently skips unknown filter fields when allowedFilters is not configured", () => { const filters: Filter[] = [ - { field: "1=1) OR (1", op: "eq", value: "x" }, + { field: "unknown_field", op: "eq", value: "x" }, ]; - expect(() => compile({}, { filters })).toThrow("not permitted"); + const { sql } = compile({}, { filters }); + expect(sql).not.toContain("unknown_field"); }); - it("rejects SQL injection attempts in filter field names", () => { + it("silently skips SQL injection attempts in filter field names", () => { + // Injection field names are not in the allowed set so they are skipped, + // meaning no unsafe SQL ever reaches ClickHouse. const injectionAttempts = [ "'; DROP TABLE analytics.events; --", "country UNION SELECT * FROM system.tables--", @@ -371,7 +376,8 @@ describe("SimpleQueryBuilder.compile", () => { ]; for (const field of injectionAttempts) { const filters: Filter[] = [{ field, op: "eq", value: "x" }]; - expect(() => compile({}, { filters })).toThrow("not permitted"); + const { sql } = compile({}, { filters }); + expect(sql).not.toContain(field); } }); @@ -399,6 +405,20 @@ describe("SimpleQueryBuilder.compile", () => { ); }); + it("skips 'href' filter on a query type that does not allow it (regression: outbound filter in mixed batch)", () => { + // When a user has an href filter active on the outbound links view, all + // batch queries receive that filter. Queries that don't declare "href" in + // allowedFilters (e.g. country, top_pages) should compile without error + // and simply omit the href condition from their WHERE clause. + const filters: Filter[] = [ + { field: "href", op: "eq", value: "https://example.com" }, + { field: "country", op: "eq", value: "US" }, + ]; + const { sql } = compile({}, { filters }); + expect(sql).not.toContain("href"); + expect(sql).toContain("country"); + }); + function whereClauseOf(sql: string): string { const whereIdx = sql.indexOf("WHERE"); expect(whereIdx).toBeGreaterThanOrEqual(0); diff --git a/packages/ai/src/query/simple-builder.ts b/packages/ai/src/query/simple-builder.ts index de693b716..a73fcb3ec 100644 --- a/packages/ai/src/query/simple-builder.ts +++ b/packages/ai/src/query/simple-builder.ts @@ -1048,6 +1048,21 @@ export class SimpleQueryBuilder { if (!filter || filter.target || filter.having) { continue; } + // Skip filters for fields not supported by this query type rather + // than throwing — in a multi-query batch, a dimension specific to + // one query type (e.g. "href" for outbound_links) should simply be + // ignored by query types that don't know about it. + const isGloballyAllowed = GLOBAL_ALLOWED_FILTERS.includes( + filter.field as (typeof GLOBAL_ALLOWED_FILTERS)[number] + ); + if ( + !( + isGloballyAllowed || + this.config.allowedFilters?.includes(filter.field) + ) + ) { + continue; + } const { clause, params: filterParams } = this.buildFilter( filter, i,