Skip to content

fix(web): validate page and limit query params on caps and space dashboard (#3567) #3574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ghazy2001 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(web): validate page and limit query params on caps and space dashboard (#3567) #3574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0dbf349
fix(web): validate page and limit query params on caps and space dash…
ghazy2001 Oct 29, 2026
f52b5cf
style(web): sort imports with biome in dashboard pagination pages (#3…
ghazy2001 Oct 30, 2026
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
49 changes: 49 additions & 0 deletions apps/web/__tests__/unit/pagination-params.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { describe, expect, it } from "vitest";
import { parseLimitParam, parsePageParam } from "../../lib/pagination";

describe("pagination params validation (#3567)", () => {
describe("parsePageParam", () => {
it("falls back to default page on missing, non-positive, or fractional input", () => {
expect(parsePageParam(undefined)).toBe(1);
expect(parsePageParam("")).toBe(1);
expect(parsePageParam("0")).toBe(1);
expect(parsePageParam("-1")).toBe(1);
expect(parsePageParam("-10")).toBe(1);
expect(parsePageParam("1.5")).toBe(1);
expect(parsePageParam("abc")).toBe(1);
expect(parsePageParam(NaN as unknown as string)).toBe(1);
});

it("parses valid positive integer pages", () => {
expect(parsePageParam("1")).toBe(1);
expect(parsePageParam("5")).toBe(5);
expect(parsePageParam(["3", "4"])).toBe(3);
});

it("supports custom default page", () => {
expect(parsePageParam("-5", 2)).toBe(2);
});
});

describe("parseLimitParam", () => {
it("falls back to default limit on missing, non-positive, or fractional input", () => {
expect(parseLimitParam(undefined, 15)).toBe(15);
expect(parseLimitParam("", 15)).toBe(15);
expect(parseLimitParam("0", 15)).toBe(15);
expect(parseLimitParam("-5", 15)).toBe(15);
expect(parseLimitParam("2.5", 15)).toBe(15);
expect(parseLimitParam("foo", 15)).toBe(15);
});

it("parses valid positive integer limits", () => {
expect(parseLimitParam("10", 15)).toBe(10);
expect(parseLimitParam("25", 15)).toBe(25);
expect(parseLimitParam(["20", "30"], 15)).toBe(20);
});

it("caps oversized limits to maxLimit", () => {
expect(parseLimitParam("500", 15, 100)).toBe(100);
expect(parseLimitParam("100", 15, 100)).toBe(100);
});
});
});
5 changes: 3 additions & 2 deletions apps/web/app/(org)/dashboard/caps/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { and, count, desc, eq, inArray, isNull, sql } from "drizzle-orm";
import { type Array, Effect } from "effect";
import type { Metadata } from "next";
import { redirect } from "next/navigation";
import { parseLimitParam, parsePageParam } from "@/lib/pagination";
import { seedThumbnailUrls } from "@/lib/seeded-thumbnails";
import { runPromise } from "@/lib/server";
import { Caps } from "./Caps";
Expand Down Expand Up @@ -133,8 +134,8 @@ export default async function CapsPage(props: PageProps<"/dashboard/caps">) {
redirect("/login");
}

const page = Number(searchParams.page) || 1;
const limit = Number(searchParams.limit) || 15;
const page = parsePageParam(searchParams.page, 1);

@greptile-apps greptile-apps Bot Oct 29, 2026

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.

P2 Next repeats the first page

The server now falls back to page 1 for fractional values, but Caps and SharedCaps still read the raw page with Number(...) || 1. Opening ?page=1.5 shows page 1's videos while marking page 1.5 as current. The next link points to page 2.5, which again loads page 1. Use the same parser in both clients, or pass the parsed page from the server.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/web/app/(org)/dashboard/caps/page.tsx
Line: 137

Comment:
**Next repeats the first page**

The server now falls back to page 1 for fractional values, but `Caps` and `SharedCaps` still read the raw page with `Number(...) || 1`. Opening `?page=1.5` shows page 1's videos while marking page 1.5 as current. The next link points to page 2.5, which again loads page 1. Use the same parser in both clients, or pass the parsed page from the server.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

const limit = parseLimitParam(searchParams.limit, 15);

const userId = user.id;
const offset = (page - 1) * limit;
Expand Down
5 changes: 3 additions & 2 deletions apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { and, count, desc, eq, inArray, isNull, or, sql } from "drizzle-orm";
import { Effect } from "effect";
import type { Metadata } from "next";
import { notFound } from "next/navigation";
import { parseLimitParam, parsePageParam } from "@/lib/pagination";
import { seedThumbnailUrls } from "@/lib/seeded-thumbnails";
import { runPromise } from "@/lib/server";
import { SharedCaps } from "./SharedCaps";
Expand Down Expand Up @@ -288,8 +289,8 @@ export default async function SharedCapsPage(props: {
}) {
const searchParams = await props.searchParams;
const params = await props.params;
const page = Number(searchParams.page) || 1;
const limit = Number(searchParams.limit) || 15;
const page = parsePageParam(searchParams.page, 1);
const limit = parseLimitParam(searchParams.limit, 15);
const user = await getCurrentUser();
if (!user) notFound();

Expand Down
24 changes: 24 additions & 0 deletions apps/web/lib/pagination.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
export function parsePageParam(
page: string | string[] | undefined,
defaultValue = 1,
): number {
const value = Array.isArray(page) ? page[0] : page;
const parsed = Number(value);

if (!Number.isInteger(parsed) || parsed < 1) return defaultValue;

@greptile-apps greptile-apps Bot Oct 29, 2026

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 Large pages still fail

parsePageParam accepts enormous pages such as 1e308. With the default limit of 15, (page - 1) * limit becomes Infinity, which both dashboards pass to .offset(). That cannot be a valid MySQL offset, so a crafted URL can still make the dashboard query fail. Bound the page so the calculated offset stays within a safe integer range, and add a test for oversized pages.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/web/lib/pagination.ts
Line: 8

Comment:
**Large pages still fail**

`parsePageParam` accepts enormous pages such as `1e308`. With the default limit of 15, `(page - 1) * limit` becomes `Infinity`, which both dashboards pass to `.offset()`. That cannot be a valid MySQL offset, so a crafted URL can still make the dashboard query fail. Bound the page so the calculated offset stays within a safe integer range, and add a test for oversized pages.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.


return parsed;
}

export function parseLimitParam(
limit: string | string[] | undefined,
defaultValue = 15,
maxLimit = 100,
): number {
const value = Array.isArray(limit) ? limit[0] : limit;
const parsed = Number(value);

if (!Number.isInteger(parsed) || parsed < 1) return defaultValue;

return Math.min(parsed, maxLimit);
}
8 changes: 2 additions & 6 deletions apps/web/lib/public-collections-policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { isEmailAllowedByRestriction } from "@cap/utils";
import { parsePageParam } from "./pagination";

export const PUBLIC_COLLECTION_PAGE_SIZE = 15;

Expand All @@ -19,12 +20,7 @@ export type PublicCollectionAccess =
export function parsePublicCollectionPage(
page: string | string[] | undefined,
): number {
const value = Array.isArray(page) ? page[0] : page;
const parsed = Number(value);

if (!Number.isInteger(parsed) || parsed < 1) return 1;

return parsed;
return parsePageParam(page, 1);
}

export function getPublicCollectionHref(id: string, page: number): string {
Expand Down
Loading