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
9 changes: 8 additions & 1 deletion web/admin/app/api/omi/releases/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { NextRequest, NextResponse } from "next/server";
import { verifyAdmin } from "@/lib/auth";
import { withRowLimit } from "@/lib/posthog";
import {
desktopQualificationFromMetadata,
desktopReleaseLifecycle,
Expand Down Expand Up @@ -106,7 +107,13 @@ async function posthogQuery(query: string): Promise<any> {
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",
},
);
Expand Down
6 changes: 4 additions & 2 deletions web/admin/app/api/omi/stats/onboarding/posthog/route.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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 (
Expand All @@ -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
`,
`),
},
};

Expand Down
9 changes: 7 additions & 2 deletions web/admin/app/api/omi/stats/profitability/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { NextRequest, NextResponse } from "next/server";

Check warning on line 1 in web/admin/app/api/omi/stats/profitability/route.ts

View workflow job for this annotation

GitHub Actions / PR Metadata Preflight

Large changed file

web/admin/app/api/omi/stats/profitability/route.ts is 809 lines; consider splitting files over 800 lines.
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";
Expand Down Expand Up @@ -184,7 +185,9 @@
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());
Expand Down Expand Up @@ -218,7 +221,9 @@
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) return null;
const raw = await response.json();
Expand Down Expand Up @@ -527,7 +532,7 @@
return { days, desktopCost, mobileCost };
}

export async function computeProfitability(opts: { days: number; desktopCost: number; mobileCost: number }): Promise<ProfitabilityPayload> {

Check warning on line 535 in web/admin/app/api/omi/stats/profitability/route.ts

View workflow job for this annotation

GitHub Actions / PR Metadata Preflight

Long function

export async function computeProfitability(opts: is 244 lines; consider extracting focused helpers over 150 lines.
const { days, desktopCost, mobileCost } = opts;

const [tokensRes, signupsRes, desktopUidsRes, mobileUidsRes, desktopActiveRes, mobileActiveRes, infraRes] = await Promise.allSettled([
Expand Down
48 changes: 48 additions & 0 deletions web/admin/lib/__tests__/posthog-no-bypass.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { readdirSync, readFileSync, statSync } from "fs";
import { join, resolve } from "path";

import { describe, expect, it } from "vitest";

// #10190 regression guard: any admin code that sends a raw HogQL query directly
// (a `kind: "HogQLQuery"` body via its own fetch, bypassing lib/posthog's
// posthogFetch chokepoint) must apply the shared `withRowLimit` guard, or it
// re-introduces the silent default-100 truncation. Routes that go through
// posthogFetch/posthogResults/cachedPosthogFetch never contain the literal, so
// they are covered automatically and don't trip this check.

const ADMIN_ROOT = resolve(__dirname, "..", "..");
// lib/posthog.ts is the definition site — it holds both the literal and the
// guard, so it passes the rule below without being a special case.
const SCAN_DIRS = ["app", "lib"];

function tsFiles(dir: string): string[] {
let out: string[] = [];
for (const entry of readdirSync(dir)) {
if (entry === "node_modules" || entry === "__tests__") continue;
const full = join(dir, entry);
if (statSync(full).isDirectory()) out = out.concat(tsFiles(full));
else if (full.endsWith(".ts") || full.endsWith(".tsx")) out.push(full);
}
return out;
}

describe("HogQL row-limit guard has no bypass", () => {
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([]);
});
});
46 changes: 46 additions & 0 deletions web/admin/lib/__tests__/posthog-row-limit.test.ts
Original file line number Diff line number Diff line change
@@ -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");
});
});
36 changes: 30 additions & 6 deletions web/admin/lib/posthog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
},
);
Expand Down
Loading