From 73400f116ea52f9e174b2c8dc8b9f6a21205b3bb Mon Sep 17 00:00:00 2001 From: Aryan Date: Tue, 21 Jul 2026 13:25:44 +0530 Subject: [PATCH 1/2] fix(admin): guard every HogQL query against PostHog's silent default LIMIT 100 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PostHog fills LIMIT 100 into any HogQL (sub)query without one and truncates silently. #10191 fixed the response-reliability charts by pinning explicit limits; the same class remained on the releases panel — 5 events x ~40-80 desktop versions exceeds 100, so per-version crash/launch/feedback counts read as 0 for most versions — and on any other LIMIT-less query. Adds withRowLimit(query, max) to lib/posthog: one explicit outer LIMIT bound via a subquery wrap. The wrap is required for UNION correctness — a trailing LIMIT after UNION ALL binds the last arm only (SELECT 1 UNION ALL SELECT 2 LIMIT 1 returns 2 rows). Applied at posthogFetch, the chokepoint both posthogResults and cachedPosthogFetch route through, so every shared-helper caller is guarded; and directly in the releases route, whose own fetch bypasses that chokepoint. Wrapping only ever adds a ceiling — a caller's tighter inner LIMIT still wins, so it never widens an intentionally small query. posthogResults' truncation observer moves from the fragile exactly-100 heuristic to the real signal now that an explicit limit is guaranteed: results at the served max (50k, PostHog's verified maximum). Test: withRowLimit unit coverage (plain query; UNION binds the whole result not the last arm; ceiling-only inner-limit preservation; custom max). typecheck + all 35 web/admin tests + lint (0 errors) green. Failure-Class: none Co-Authored-By: Claude Opus 4.8 --- web/admin/app/api/omi/releases/route.ts | 9 +++- .../lib/__tests__/posthog-row-limit.test.ts | 46 +++++++++++++++++++ web/admin/lib/posthog.ts | 36 ++++++++++++--- 3 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 web/admin/lib/__tests__/posthog-row-limit.test.ts diff --git a/web/admin/app/api/omi/releases/route.ts b/web/admin/app/api/omi/releases/route.ts index 2d0417c0479..dbcf708703c 100644 --- a/web/admin/app/api/omi/releases/route.ts +++ b/web/admin/app/api/omi/releases/route.ts @@ -1,5 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; import { verifyAdmin } from "@/lib/auth"; +import { withRowLimit } from "@/lib/posthog"; import { desktopQualificationFromMetadata, desktopReleaseLifecycle, @@ -106,7 +107,13 @@ async function posthogQuery(query: string): Promise { Authorization: `Bearer ${POSTHOG_API_KEY}`, "Content-Type": "application/json", }, - body: JSON.stringify({ query: { kind: "HogQLQuery", query } }), + // This route has its own fetch (bypasses lib/posthog's posthogFetch), so + // it applies the same shared row-limit guard directly — the version×event + // query has no LIMIT and 5 events × ~40-80 versions exceeds the silent + // default 100, dropping per-version counts to 0 (#10190). + body: JSON.stringify({ + query: { kind: "HogQLQuery", query: withRowLimit(query) }, + }), cache: "no-store", }, ); diff --git a/web/admin/lib/__tests__/posthog-row-limit.test.ts b/web/admin/lib/__tests__/posthog-row-limit.test.ts new file mode 100644 index 00000000000..127a1090e4d --- /dev/null +++ b/web/admin/lib/__tests__/posthog-row-limit.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; + +import { POSTHOG_SERVED_MAX_ROWS, withRowLimit } from "../posthog"; + +// #10190: PostHog silently fills LIMIT 100 into any HogQL (sub)query without +// one. withRowLimit binds one explicit outer LIMIT so the default cap can never +// truncate, and wraps in a subquery so a UNION's total (not just its last arm) +// is bounded. +describe("withRowLimit", () => { + it("binds an explicit outer LIMIT at the served max by default", () => { + const out = withRowLimit( + "SELECT version, count() FROM events GROUP BY version", + ); + expect(out).toContain(`LIMIT ${POSTHOG_SERVED_MAX_ROWS}`); + // The served max is 50k, verified live (LIMIT 100000 returns 50000). + expect(POSTHOG_SERVED_MAX_ROWS).toBe(50_000); + }); + + it("wraps in a subquery so the LIMIT binds the whole UNION, not the last arm", () => { + const union = "SELECT 1 AS a UNION ALL SELECT 2 AS a"; + const out = withRowLimit(union); + // The union must sit inside a subquery; the LIMIT must come after the close + // paren, not inline with the last arm (which would bind that arm only). + expect(out).toMatch( + /SELECT \* FROM \(\s*[\s\S]*UNION ALL[\s\S]*\)\s*AS _row_limit_guard\s*LIMIT/, + ); + const limitPos = out.lastIndexOf("LIMIT"); + const closeParenPos = out.lastIndexOf(")"); + expect(limitPos).toBeGreaterThan(closeParenPos); + }); + + it("is ceiling-only: a caller's tighter inner LIMIT is preserved inside the wrap", () => { + const out = withRowLimit("SELECT * FROM events LIMIT 10"); + // The inner LIMIT 10 survives (inside the subquery); the outer is just a cap. + expect(out).toContain("LIMIT 10"); + expect(out).toContain(`LIMIT ${POSTHOG_SERVED_MAX_ROWS}`); + // Inner limit appears before the guard's outer limit. + expect(out.indexOf("LIMIT 10")).toBeLessThan( + out.lastIndexOf(`LIMIT ${POSTHOG_SERVED_MAX_ROWS}`), + ); + }); + + it("honors a custom max", () => { + expect(withRowLimit("SELECT 1", 500)).toContain("LIMIT 500"); + }); +}); diff --git a/web/admin/lib/posthog.ts b/web/admin/lib/posthog.ts index 6d3763b147c..9c173804e6d 100644 --- a/web/admin/lib/posthog.ts +++ b/web/admin/lib/posthog.ts @@ -21,6 +21,26 @@ import { getDb } from "@/lib/firebase/admin"; const CACHE_COLLECTION = "admin_stats_cache"; const SOFT_TTL_MS = 30 * 60 * 1000; // serve cached without re-querying for 30 min +// PostHog's query API fills `LIMIT 100` into any HogQL (sub)query that carries +// none and truncates silently — this cut the newest days off the +// response-reliability charts (#10191) and drops per-version rows on the +// releases panel (#10190). 50_000 is PostHog's served maximum (verified live: +// `LIMIT 100000` returns exactly 50000 rows). +export const POSTHOG_SERVED_MAX_ROWS = 50_000; + +// Guard against the silent default cap by binding one explicit outer LIMIT to +// the whole result. Wrapping in a subquery is required for correctness with +// `UNION ALL`, where a trailing `LIMIT` binds to the last arm only (verified: +// `SELECT 1 UNION ALL SELECT 2 LIMIT 1` returns 2 rows). Wrapping only ever +// adds a ceiling: a caller's tighter inner `LIMIT` still wins, so this never +// widens an intentionally small query. +export function withRowLimit( + query: string, + max: number = POSTHOG_SERVED_MAX_ROWS, +): string { + return `SELECT * FROM (\n${query}\n) AS _row_limit_guard\nLIMIT ${max}`; +} + type CacheDoc = { payload: string; freshAt: number }; async function readCache( @@ -67,7 +87,9 @@ export async function posthogFetch( "Content-Type": "application/json", Accept: "application/json", }, - body: JSON.stringify({ query: { kind: "HogQLQuery", query } }), + body: JSON.stringify({ + query: { kind: "HogQLQuery", query: withRowLimit(query) }, + }), }); if (res.status !== 429 || attempt === maxRetries) return res; const waitMs = 800 * (attempt + 1) + Math.floor(Math.random() * 300); @@ -146,13 +168,15 @@ export async function posthogResults( } const raw = await res.json(); const results = Array.isArray(raw.results) ? raw.results : []; - // PostHog fills LIMIT 100 into any HogQL (sub)query that has none and - // truncates silently — exactly 100 rows from a LIMIT-less query is the - // signature of that cap (it cut the newest days off response-reliability). - if (results.length === 100 && !/\blimit\b/i.test(query)) { + // posthogFetch now binds an explicit LIMIT of POSTHOG_SERVED_MAX_ROWS to + // every query, so the silent default-100 cap can no longer truncate. The + // remaining ceiling is PostHog's served maximum: a result at that count is + // itself truncated and the caller should widen its window or paginate. + if (results.length >= POSTHOG_SERVED_MAX_ROWS) { console.warn( - "PostHog returned exactly 100 rows for a query without LIMIT — likely truncated by the default cap", + "PostHog returned the served-max row count — result is truncated at the ceiling", { + servedMax: POSTHOG_SERVED_MAX_ROWS, querySnippet: query.replace(/\s+/g, " ").trim().slice(0, 160), }, ); From 69dc5ddf957d090c4f306fc9a7f1a98daf10ff6f Mon Sep 17 00:00:00 2001 From: Aryan Date: Tue, 21 Jul 2026 16:57:36 +0530 Subject: [PATCH 2/2] fix(admin): guard the remaining direct PostHog query routes + add a no-bypass ratchet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to review on #10190: the row-limit guard must cover every admin HogQL request, but two routes have their own fetch that bypasses lib/posthog's posthogFetch chokepoint and were still sending unwrapped queries — profitability (fetchDesktopActivePerDay's GROUP BY day has no LIMIT; the other query's ad-hoc LIMIT 500000 overstates PostHog's 50k served max) and onboarding. Both now apply withRowLimit directly, matching the releases route. Adds a regression ratchet (posthog-no-bypass.test.ts): any file sending a raw 'kind: HogQLQuery' body must also apply withRowLimit, so a future route with its own fetch can't silently reintroduce the default-100 truncation. The test fails on exactly the state this commit fixes. Kept the edits minimal and style-matched rather than prettier-reformatting the whole files — web/admin has no prettier CI check (only web/frontend does), and these files were already non-compliant on main, so a wholesale reformat would be hundreds of lines of unrelated churn. Co-Authored-By: Claude Opus 4.8 --- .../api/omi/stats/onboarding/posthog/route.ts | 6 ++- .../app/api/omi/stats/profitability/route.ts | 9 +++- .../lib/__tests__/posthog-no-bypass.test.ts | 48 +++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 web/admin/lib/__tests__/posthog-no-bypass.test.ts diff --git a/web/admin/app/api/omi/stats/onboarding/posthog/route.ts b/web/admin/app/api/omi/stats/onboarding/posthog/route.ts index 3fffd1f7da3..0b2bd0bbbfb 100644 --- a/web/admin/app/api/omi/stats/onboarding/posthog/route.ts +++ b/web/admin/app/api/omi/stats/onboarding/posthog/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { verifyAdmin } from '@/lib/auth'; import { getPayload, setPayload } from '@/lib/payload-cache'; +import { withRowLimit } from '@/lib/posthog'; export const dynamic = 'force-dynamic'; export const maxDuration = 3600; @@ -117,7 +118,8 @@ export async function computeOnboarding(days: number) { const body = { query: { kind: 'HogQLQuery', - query: ` + // Own fetch (bypasses posthogFetch); guard directly (#10190). + query: withRowLimit(` WITH entrant_actors AS ( SELECT actor_id FROM ( @@ -142,7 +144,7 @@ export async function computeOnboarding(days: number) { AND COALESCE(person_id, distinct_id) IN (SELECT actor_id FROM entrant_actors) GROUP BY actor_id, event LIMIT 10000 - `, + `), }, }; diff --git a/web/admin/app/api/omi/stats/profitability/route.ts b/web/admin/app/api/omi/stats/profitability/route.ts index 00f2fee012c..0d5c9bc4798 100644 --- a/web/admin/app/api/omi/stats/profitability/route.ts +++ b/web/admin/app/api/omi/stats/profitability/route.ts @@ -1,5 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; import { verifyAdmin } from "@/lib/auth"; +import { withRowLimit } from "@/lib/posthog"; import { getOptionalStripe } from "@/lib/stripe"; import { getAdminAuth, getDb } from "@/lib/firebase/admin"; import { getPayload, setPayload } from "@/lib/payload-cache"; @@ -184,7 +185,9 @@ async function fetchDesktopUidsFromPostHog(days: number): Promise | const response = await fetch(`${host}/api/projects/${projectId}/query/`, { method: "POST", headers: { Authorization: `Bearer ${apiKey}`, "Content-Type": "application/json" }, - body: JSON.stringify({ query: { kind: "HogQLQuery", query } }), + // Own fetch (bypasses lib/posthog's posthogFetch); apply the shared + // row-limit guard directly so this can't silently truncate (#10190). + body: JSON.stringify({ query: { kind: "HogQLQuery", query: withRowLimit(query) } }), }); if (!response.ok) { console.error("PostHog desktop uids failed:", response.status, await response.text()); @@ -218,7 +221,9 @@ async function fetchDesktopActivePerDay(days: number): Promise { + it("every file that sends a raw HogQLQuery also applies withRowLimit", () => { + const offenders: string[] = []; + for (const scanDir of SCAN_DIRS) { + for (const file of tsFiles(join(ADMIN_ROOT, scanDir))) { + const src = readFileSync(file, "utf8"); + if (!/HogQLQuery/.test(src)) continue; + if (!/withRowLimit/.test(src)) { + offenders.push(file.slice(ADMIN_ROOT.length + 1)); + } + } + } + expect( + offenders, + `These files send a raw HogQLQuery without the withRowLimit guard (see #10190). ` + + `Route the fetch through lib/posthog's posthogFetch/posthogResults, or wrap the query with withRowLimit:\n` + + offenders.join("\n"), + ).toEqual([]); + }); +});