mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-22 07:02:16 +03:00
fix(gamification): validate leaderboard limit/offset before the SQLite bind (#11059)
⭐5 — LIMIT negativo = "sem limite" no SQLite: ?limit=-1 retornava a leaderboard inteira em endpoint management autenticado; ceil por Math.min só no upper. Duas camadas: route rejeita não-inteiro/fora de range com 400 (mesmo contrato de parseListLimit), getTopN clampeia como backstop defense-in-depth + exporta LEADERBOARD_MAX_LIMIT. TDD red→green, 5 casos novos, 84/84 suíte gamification. Fecha #11058.
This commit is contained in:
@@ -38,7 +38,14 @@ export async function GET(request: NextRequest) {
|
||||
|
||||
const url = new URL(request.url);
|
||||
const scope: LeaderboardScope = (url.searchParams.get("scope") || "global") as LeaderboardScope;
|
||||
const limit = Number(url.searchParams.get("limit") || 100);
|
||||
const rawLimit = url.searchParams.get("limit");
|
||||
const limit = rawLimit === null ? 100 : Number(rawLimit);
|
||||
if (!Number.isInteger(limit) || limit < 1 || limit > 200) {
|
||||
return NextResponse.json(
|
||||
{ error: "'limit' must be an integer between 1 and 200" },
|
||||
{ status: 400, headers: CORS_HEADERS }
|
||||
);
|
||||
}
|
||||
|
||||
const entries = await getTopN(scope, limit);
|
||||
|
||||
|
||||
@@ -18,10 +18,18 @@ export async function GET(request: NextRequest) {
|
||||
|
||||
const url = new URL(request.url);
|
||||
const scope = (url.searchParams.get("scope") || "global") as LeaderboardScope;
|
||||
const limit = Number(url.searchParams.get("limit") || 50);
|
||||
const rawLimit = url.searchParams.get("limit");
|
||||
const limit = rawLimit === null ? 50 : Number(rawLimit);
|
||||
const apiKeyId = url.searchParams.get("apiKeyId");
|
||||
|
||||
const entries = await getTopN(scope, Math.min(limit, 200));
|
||||
if (!Number.isInteger(limit) || limit < 1 || limit > 200) {
|
||||
return NextResponse.json(
|
||||
{ error: "'limit' must be an integer between 1 and 200" },
|
||||
{ status: 400, headers: CORS_HEADERS }
|
||||
);
|
||||
}
|
||||
|
||||
const entries = await getTopN(scope, limit);
|
||||
let myRank: number | null = null;
|
||||
let neighbors = null;
|
||||
|
||||
|
||||
@@ -131,13 +131,24 @@ export function getRank(apiKeyId: string, scope: string): number {
|
||||
return rankRow.rank;
|
||||
}
|
||||
|
||||
export const LEADERBOARD_MAX_LIMIT = 200;
|
||||
|
||||
export function getTopN(scope: string, limit: number, offset: number = 0): LeaderboardRow[] {
|
||||
// Guard the SQLite LIMIT/OFFSET bind. A negative LIMIT means "no limit" in
|
||||
// SQLite (returns the whole table), and a non-integer throws a datatype
|
||||
// mismatch, so an unvalidated `?limit` on a caller route (e.g. the leaderboard
|
||||
// endpoints) could read the entire leaderboard or 500. Clamp to a coherent
|
||||
// range here as a defense-in-depth backstop, independent of route validation.
|
||||
const safeLimit = Number.isFinite(limit)
|
||||
? Math.min(Math.max(Math.trunc(limit), 0), LEADERBOARD_MAX_LIMIT)
|
||||
: 0;
|
||||
const safeOffset = Number.isFinite(offset) ? Math.max(Math.trunc(offset), 0) : 0;
|
||||
const rows = db()
|
||||
.prepare(
|
||||
`SELECT api_key_id, scope, score, updated_at FROM leaderboard
|
||||
WHERE scope = ? ORDER BY score DESC LIMIT ? OFFSET ?`
|
||||
)
|
||||
.all(scope, limit, offset) as Array<{
|
||||
.all(scope, safeLimit, safeOffset) as Array<{
|
||||
api_key_id: string;
|
||||
scope: string;
|
||||
score: number;
|
||||
|
||||
58
tests/unit/gamification/leaderboard-limit-validation.test.ts
Normal file
58
tests/unit/gamification/leaderboard-limit-validation.test.ts
Normal file
@@ -0,0 +1,58 @@
|
||||
import { describe, it, before, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { getTopN, LEADERBOARD_MAX_LIMIT } from "../../../src/lib/db/gamification";
|
||||
import { getDbInstance } from "../../../src/lib/db/core";
|
||||
|
||||
// Regression for the unvalidated `?limit` that reached the SQLite LIMIT bind on
|
||||
// the leaderboard endpoints. In SQLite a negative LIMIT means "no limit", so a
|
||||
// caller passing limit=-1 would read the entire leaderboard; a non-integer would
|
||||
// throw a datatype mismatch. getTopN must clamp the bind as a backstop.
|
||||
describe("getTopN limit/offset clamping", () => {
|
||||
const scope = "global";
|
||||
const keys: string[] = [];
|
||||
|
||||
before(() => {
|
||||
const db = getDbInstance();
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const k = `test-lb-${Date.now()}-${i}`;
|
||||
keys.push(k);
|
||||
db
|
||||
.prepare(
|
||||
"INSERT OR REPLACE INTO leaderboard (api_key_id, scope, score, updated_at) VALUES (?, ?, ?, ?)"
|
||||
)
|
||||
.run(k, scope, 100 - i, new Date().toISOString());
|
||||
}
|
||||
});
|
||||
|
||||
after(() => {
|
||||
const db = getDbInstance();
|
||||
for (const k of keys) {
|
||||
db.prepare("DELETE FROM leaderboard WHERE api_key_id = ?").run(k);
|
||||
}
|
||||
});
|
||||
|
||||
it("returns at most the requested number of rows", () => {
|
||||
assert.equal(getTopN(scope, 2).length, 2);
|
||||
});
|
||||
|
||||
it("treats a negative limit as empty, never as unbounded", () => {
|
||||
// Pre-fix this returned every row (SQLite LIMIT -1 == no limit).
|
||||
assert.equal(getTopN(scope, -1).length, 0);
|
||||
assert.equal(getTopN(scope, -100).length, 0);
|
||||
});
|
||||
|
||||
it("treats a non-integer limit as empty instead of throwing", () => {
|
||||
assert.equal(getTopN(scope, Number.NaN).length, 0);
|
||||
});
|
||||
|
||||
it("caps the limit at LEADERBOARD_MAX_LIMIT", () => {
|
||||
const rows = getTopN(scope, LEADERBOARD_MAX_LIMIT + 5000);
|
||||
assert.ok(rows.length <= LEADERBOARD_MAX_LIMIT);
|
||||
});
|
||||
|
||||
it("never binds a negative offset", () => {
|
||||
// Would throw or behave oddly if a negative offset reached SQLite.
|
||||
assert.doesNotThrow(() => getTopN(scope, 2, -10));
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user