fix(profiles): escape search (SQLi), guard bare filter names, robust date parse

Review fixes on the profiles behavioural filter:

- SECURITY (critical): escape `search` in the fuzzy ILIKE clause (list + count)
  with sqlstring.escape — was interpolated raw, allowing SQL injection.
- SECURITY (major): reject non-identifier bare event-filter names in
  eventFilterClauses before interpolating (column names can't be string-escaped).
- search + behavioural now applies the exact-window clamp + count threshold via
  buildBehavioralV2Subquery, so results match the count (was silently dropped).
- last-seen-picker: parse persisted yyyy-MM-dd HH:mm:ss with date-fns instead of
  new Date() (Safari returned Invalid Date, leaving the picker empty).
- drop the unrelated CLICKHOUSE_MAX_EXECUTION_TIME cap (separate concern).

Author: ayushjhanwar-png

.env.example

@@ -52,14 +52,6 @@ ENABLE_TRPC_CACHE=""
 # 10 ("Too many simultaneous queries"). Unset = leave CH's default untouched.
 CLICKHOUSE_QUERY_LIMIT=""
 
-# Hard default read timeout (seconds) for every SELECT through the app's CH
-# client. Without it, a read with no per-query max_execution_time runs
-# UNBOUNDED — a Profiles "did event X" behavioural filter once ran 45 minutes
-# from a laptop pointed at prod CH, spiking cluster CPU. Default 60. A per-query
-# max_execution_time still wins for legit long reads; prod CLICKHOUSE_SETTINGS
-# (spread last) overrides this. Inserts use their own 300s in the insert proxy.
-CLICKHOUSE_MAX_EXECUTION_TIME="60"
-
 # PROFILES PAGE — BEHAVIORAL FILTER v2 ROUTING
 # When the profiles-page behavioural filter ("users who did event X where
 # properties.k=v in last N days") runs, route the subquery to

apps/start/src/components/profiles/last-seen-picker.tsx

@@ -24,6 +24,17 @@ interface Props {
 
 const DATE_FMT = 'MMM d, yyyy';
 const DATETIME_FMT = 'MMM d, yyyy, hh:mm a';
+const DB_FMT = 'yyyy-MM-dd HH:mm:ss';
+
+// Parse a persisted DB-format string with date-fns (not `new Date(...)`, which
+// is browser-dependent for space-separated `yyyy-MM-dd HH:mm:ss` — Safari yields
+// Invalid Date). Returns undefined for null/invalid so the picker stays empty
+// rather than showing NaN.
+function parseDb(s: string | null): Date | undefined {
+  if (!s) return undefined;
+  const d = parse(s, DB_FMT, new Date());
+  return isValid(d) ? d : undefined;
+}
 
 // Copy a calendar-picked day onto an existing datetime, preserving the time
 // (react-day-picker returns midnight). Falls back to a default HH:mm.
@@ -56,18 +67,14 @@ export function LastSeenPicker({
   const [mode, setMode] = useState<Mode>(
     startDate && !endDate ? 'since' : 'fixed',
   );
-  const [from, setFrom] = useState<Date | undefined>(
-    startDate ? new Date(startDate) : undefined,
-  );
-  const [to, setTo] = useState<Date | undefined>(
-    endDate ? new Date(endDate) : undefined,
-  );
+  const [from, setFrom] = useState<Date | undefined>(parseDb(startDate));
+  const [to, setTo] = useState<Date | undefined>(parseDb(endDate));
   const [enableTime, setEnableTime] = useState(false);
 
   const sync = () => {
     setMode(startDate && !endDate ? 'since' : 'fixed');
-    setFrom(startDate ? new Date(startDate) : undefined);
-    setTo(endDate ? new Date(endDate) : undefined);
+    setFrom(parseDb(startDate));
+    setTo(parseDb(endDate));
   };
 
   const canApply = mode === 'fixed' ? !!from && !!to : !!from;

docs/profiles-v2-behavioral-filter.md

@@ -131,14 +131,12 @@ count above the identified-only base.
 
 - `PROFILES_BEHAVIORAL_V2_PROJECTS` — comma-separated allowlist (per-project rollout).
 - `PROFILES_BEHAVIORAL_V2_START_DATE` — earliest window start v2 may serve.
-- `CLICKHOUSE_MAX_EXECUTION_TIME` — app CH client cap (default 60 s) so a stray
-  read can't run for minutes. Prod's `CLICKHOUSE_SETTINGS` (40 s) still wins.
 
 ## Files
 
 - `packages/db/src/services/profile.service.ts` — routing, two-step, clamp,
   operators, count.
-- `packages/db/src/clickhouse/client.ts` — v2 in `TABLE_NAMES`; `max_execution_time`.
+- `packages/db/src/clickhouse/client.ts` — v2 in `TABLE_NAMES`.
 - `packages/trpc/src/routers/profile.ts` — `eventCount` input.
 - `apps/start/.../profiles._tabs.identified.tsx` — query wiring, 15-day default,
   all-profiles (no is_external).

packages/db/src/clickhouse/client.ts

@@ -111,20 +111,8 @@ function getClickhouseSettings(): ClickHouseSettings {
       ? rawQueryLimit
       : undefined;
 
-  // Hard default read timeout. Without this, a read query with no per-query
-  // max_execution_time runs UNBOUNDED — a Profiles "did event X" behavioral
-  // filter (id IN (SELECT … FROM events …) + FINAL) ran 45 min from a laptop
-  // against prod CH (public IP, avnadmin), peaking 3.5 GB and spiking cluster
-  // CPU, because nothing capped it. This floor caps every read via this client.
-  // Inserts override to 300 in the `insert` proxy; a per-query max_execution_time
-  // still wins for legitimately long reads; prod CLICKHOUSE_SETTINGS can override.
-  const rawMaxExec = process.env.CLICKHOUSE_MAX_EXECUTION_TIME?.trim();
-  const maxExecutionTime =
-    rawMaxExec && Number.isFinite(Number(rawMaxExec)) ? Number(rawMaxExec) : 60;
-
   return {
     date_time_input_format: 'best_effort',
-    max_execution_time: maxExecutionTime,
     ...(queryLimit ? { max_concurrent_queries_for_user: queryLimit } : {}),
     ...(!process.env.CLICKHOUSE_SETTINGS_REMOVE_CONVERT_ANY_JOIN
       ? {

packages/db/src/services/profile.service.ts

@@ -393,6 +393,7 @@ function buildBehavioralV2Subquery(
   startDate: string | null | undefined,
   endDate: string | null | undefined,
   tz: string,
+  eventCount: IProfileEventCount | undefined,
 ): string {
   const where = buildBehavioralV2WhereClause(
     projectId,
@@ -403,6 +404,14 @@ function buildBehavioralV2Subquery(
     endDate,
     tz,
   );
+  // Apply the same exact-window clamp + count threshold as the two-step path, so
+  // the search+behavioral list (which routes through here, not the two-step)
+  // matches the count and honours "OP N times". Without a HAVING, a plain
+  // DISTINCT is enough and cheaper.
+  const having = buildBehavioralV2Having(startDate, endDate, tz, eventCount);
+  if (having) {
+    return `SELECT profile_id FROM ${TABLE_NAMES.profile_event_property_summary_v2} WHERE ${where} GROUP BY profile_id ${having}`;
+  }
   return `SELECT DISTINCT profile_id FROM ${TABLE_NAMES.profile_event_property_summary_v2} WHERE ${where}`;
 }
 
@@ -539,9 +548,16 @@ function eventFilterClauses(filters: IChartEventFilter[]): string[] {
   const out: string[] = [];
   for (const f of filters) {
     if (!f.name || !f.value?.length) continue;
-    const col = f.name.startsWith('properties.')
-      ? `properties[${sqlstring.escape(f.name.replace(/^properties\./, ''))}]`
-      : f.name;
+    let col: string;
+    if (f.name.startsWith('properties.')) {
+      col = `properties[${sqlstring.escape(f.name.replace(/^properties\./, ''))}]`;
+    } else {
+      // Bare event column (country/os/path/…). It's a SQL identifier, not a
+      // string literal, so it can't be sqlstring.escape'd — reject anything that
+      // isn't a plain identifier to block injection via a crafted filter name.
+      if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(f.name)) continue;
+      col = f.name;
+    }
     const inList = f.value.map((v) => sqlstring.escape(String(v))).join(',');
     if (f.operator === 'isNot') {
       out.push(`${col} NOT IN (${inList})`);
@@ -702,7 +718,10 @@ export async function getProfileList({
     } else if (opts.searchMode === 'fuzzy') {
       // Substring name/email search. ILIKE '%x%' can't use any index (the bloom
       // filters only help exact/token matches), so it scans — inherently slow.
-      sb.where.search = `(email ILIKE '%${search}%' OR first_name ILIKE '%${search}%' OR last_name ILIKE '%${search}%')`;
+      // Escape the user input: sqlstring.escape wraps + escapes so a crafted
+      // `search` can't break out of the string literal (SQL injection).
+      const like = sqlstring.escape(`%${search}%`);
+      sb.where.search = `(email ILIKE ${like} OR first_name ILIKE ${like} OR last_name ILIKE ${like})`;
     }
     if (isExternal !== undefined) {
       sb.where.external = `is_external = ${isExternal ? 'true' : 'false'}`;
@@ -718,7 +737,7 @@ export async function getProfileList({
       // either way, so "Last seen" (profiles.created_at) and sorting are
       // unchanged. Falls back to events for anything v2 can't serve.
       if (canRouteBehavioralToV2(projectId, filters ?? [], range, startDate)) {
-        sb.where.behavioral = `id IN (${buildBehavioralV2Subquery(projectId, eventNames, filters ?? [], range, startDate, endDate, tz)})`;
+        sb.where.behavioral = `id IN (${buildBehavioralV2Subquery(projectId, eventNames, filters ?? [], range, startDate, endDate, tz, eventCount)})`;
       } else {
         const names = eventNames.map((e) => sqlstring.escape(e)).join(',');
         const parts = [
@@ -924,7 +943,9 @@ export async function getProfileListCount({
   sb.where.project_id = `project_id = ${sqlstring.escape(projectId)}`;
   sb.groupBy.project_id = 'project_id';
   if (search) {
-    sb.where.search = `(email ILIKE '%${search}%' OR first_name ILIKE '%${search}%' OR last_name ILIKE '%${search}%')`;
+    // Escape user input — see getProfileList fuzzy branch (SQL injection).
+    const like = sqlstring.escape(`%${search}%`);
+    sb.where.search = `(email ILIKE ${like} OR first_name ILIKE ${like} OR last_name ILIKE ${like})`;
   }
   if (isExternal !== undefined) {
     sb.where.external = `is_external = ${isExternal ? 'true' : 'false'}`;