Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 29 additions & 9 deletions packages/ai/src/query/simple-builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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--",
Expand All @@ -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);
}
});

Expand Down Expand Up @@ -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);
Expand Down
15 changes: 15 additions & 0 deletions packages/ai/src/query/simple-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,21 @@
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(

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at buildWhereClause (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1033:29) at buildStandardQuery (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:728:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:159:19)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at buildWhereClause (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1033:29) at buildStandardQuery (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:728:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:159:19)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at buildWhereClause (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1033:29) at buildStandardQuery (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:728:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:159:19)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at buildWhereClause (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1033:29) at buildStandardQuery (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:728:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:159:19)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at compile (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:647:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:144:28)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at compile (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:647:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:144:28)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at buildWhereClause (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1033:29) at buildStandardQuery (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:728:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:144:28)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at compile (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:647:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:131:28)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at compile (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:647:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:131:28)

Check failure on line 1055 in packages/ai/src/query/simple-builder.ts

View workflow job for this annotation

GitHub Actions / Test

TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function. (In 'GLOBAL_ALLOWED_FILTERS.includes(filter.field)'

at buildWhereClauseFromFilters (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1055:54) at buildWhereClause (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:1033:29) at buildStandardQuery (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.ts:728:29) at <anonymous> (/home/runner/_work/Databuddy/Databuddy/packages/ai/src/query/simple-builder.test.ts:131:28)
filter.field as (typeof GLOBAL_ALLOWED_FILTERS)[number]
);
if (
!(
isGloballyAllowed ||
this.config.allowedFilters?.includes(filter.field)
)
) {
continue;
Comment on lines +1055 to +1064

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Set.prototype.includes does not exist — TypeError on every request with filters

GLOBAL_ALLOWED_FILTERS is declared as new Set([...]) (line 44). Set has .has(), not .includes(). Calling .includes() on a Set throws TypeError: GLOBAL_ALLOWED_FILTERS.includes is not a function at runtime, which would crash every query that carries filters — a regression worse than the original throw. The pre-existing isFilterFieldAllowed helper (line 61-68) already performs this exact check correctly using .has(). The entire new block should be replaced with a single call to that helper:

if (!isFilterFieldAllowed(this.config, filter.field)) {
    continue;
}

The type assertion as (typeof GLOBAL_ALLOWED_FILTERS)[number] is also unsound for a SetSet<string>[number] resolves to undefined — but it is superseded by the runtime crash anyway.

}
const { clause, params: filterParams } = this.buildFilter(
filter,
i,
Expand Down
Loading