P1-P3 admin cleanup: rank guard, SSRF guard, count caching, dead code removal, pagination rewrite

- P1: new src/lib/admin/rank-guard.ts with tryGuardRank/guardRank + RankGuardTarget/Result; extracted from users.ts; applied to admin-bans (create/lift + rank redirect) and admin-badges (caps + rank guard + error surfacing); import-url-safety.ts SSRF guard wired into clone save+icon fetch; logAudit await in help-tickets; antiddos mutations SETTINGS_EDIT; markAllAlertsRead NOTIFICATIONS_EDIT; rankError/saveError in all 25 message files; tests rew/new for bans/badges/rank-guard/import-url-safety

- P2: ticket-inbox merged pagination reworked to single UNION ALL SQL with LIMIT/OFFSET (avoids O(offset) materialization); getCachedAdminCount with cacheFiltered option applied to devops/errors/photos/items/guilds/help-tickets/moderation pages; admin-theme catches→?error=1+banner+25 locales; logger in silent catches (rare-values/writeable-boxes); catalog-items-loader CAST→plain equality; count caching helper extended with opt-in cacheFiltered

- P3: removed dead modules is-staff/notice/recent-furni-resync/log-filters plus load-user-sanctions/user-sanction-timeline; removed orphaned CATALOG_PAGE_COLUMNS re-export; removed getFurniIconUrl from studio-furni-icons

Build: lint 0, typecheck 0, 3480 tests pass, i18n:check exit 0, colors exit 0. Circular chunk warning (9452/10535/66463) is webpack code-splitting artifact; build succeeds.

src/lib/admin-operations-contract.test.ts

@@ -73,6 +73,7 @@ const ACTION_GATES: Array<[string, string]> = [
 	["admin-ads.ts", "PERMS.PAGES_EDIT"],
 	["admin-vouchers.ts", "PERMS.SHOP_EDIT"],
 	["admin-alerts.ts", "PERMS.NOTIFICATIONS_EDIT"],
+	["admin-antiddos.ts", "PERMS.SETTINGS_EDIT"],
 	["commandocentrum.ts", "PERMS.RCON_EXECUTE"],
 	["translations.ts", "PERMS.SETTINGS_EDIT"],
 	["tickets.ts", "PERMS.TICKETS_EDIT"],

src/lib/admin/is-staff.test.ts

@@ -1,14 +0,0 @@
-import { describe, expect, it } from "vitest";
-import { isStaff } from "./is-staff";
-
-describe("isStaff", () => {
-	it("is true at or above the min staff rank", () => {
-		expect(isStaff(7, 7)).toBe(true);
-		expect(isStaff(10, 7)).toBe(true);
-	});
-
-	it("is false below the min staff rank", () => {
-		expect(isStaff(6, 7)).toBe(false);
-		expect(isStaff(1, 7)).toBe(false);
-	});
-});

src/lib/admin/is-staff.ts

@@ -1,4 +0,0 @@
-/** AtomCMS housekeeping gate: staff are users with rank >= min_staff_rank. */
-export function isStaff(rank: number, minStaffRank: number): boolean {
-	return rank >= minStaffRank;
-}

src/lib/admin/log-filters.test.ts

@@ -1,39 +0,0 @@
-import { describe, expect, it } from "vitest";
-import { buildStaffActivityWhere } from "./log-filters";
-
-describe("buildStaffActivityWhere", () => {
-	it("returns an empty where when no filters are set", () => {
-		expect(buildStaffActivityWhere({})).toEqual({});
-	});
-
-	it("builds an OR search across action, description and IP", () => {
-		const where = buildStaffActivityWhere({ q: "ban " });
-		expect(where.OR).toEqual([
-			{ action: { contains: "ban" } },
-			{ description: { contains: "ban" } },
-			{ ipAddress: { contains: "ban" } },
-		]);
-		expect(where.OR?.[0]).toEqual({ action: { contains: "ban" } });
-	});
-
-	it("sets userId when staffId is provided", () => {
-		const where = buildStaffActivityWhere({ staffId: 42 });
-		expect(where.userId).toBe(BigInt(42));
-	});
-
-	it("uses a prefix match for authorization-only", () => {
-		expect(buildStaffActivityWhere({ authorizationOnly: true })).toEqual({
-			action: { startsWith: "permission." },
-		});
-	});
-
-	it("uses a contains match for a specific action", () => {
-		expect(buildStaffActivityWhere({ action: "ban" })).toEqual({
-			action: { contains: "ban" },
-		});
-	});
-
-	it("ignores whitespace-only search terms", () => {
-		expect(buildStaffActivityWhere({ q: "   " })).toEqual({});
-	});
-});

src/lib/admin/log-filters.ts

@@ -1,28 +0,0 @@
-export interface StaffActivityFilters {
-	q?: string;
-	staffId?: number | null;
-	action?: string | null;
-	authorizationOnly?: boolean;
-}
-
-interface StaffActivityWhere {
-	OR?: Array<Record<string, { contains?: string }>>;
-	userId?: bigint;
-	action?: { startsWith: string } | { contains: string };
-}
-
-export function buildStaffActivityWhere(
-	filters: StaffActivityFilters,
-): StaffActivityWhere {
-	const where: StaffActivityWhere = {};
-	if (filters.q?.trim())
-		where.OR = [
-			{ action: { contains: filters.q.trim() } },
-			{ description: { contains: filters.q.trim() } },
-			{ ipAddress: { contains: filters.q.trim() } },
-		];
-	if (filters.staffId) where.userId = BigInt(filters.staffId);
-	if (filters.authorizationOnly) where.action = { startsWith: "permission." };
-	else if (filters.action) where.action = { contains: filters.action };
-	return where;
-}

src/lib/admin/notice.test.ts

@@ -1,26 +0,0 @@
-import { describe, expect, it } from "vitest";
-import { adminMutationNotice } from "./notice";
-
-describe("adminMutationNotice", () => {
-	it("returns a danger notice when there is an error", () => {
-		expect(adminMutationNotice({ error: "boom" })).toEqual({
-			tone: "danger",
-			label: "Error",
-			message: "The operation could not be completed.",
-		});
-	});
-	it("returns a success notice when saved is 1", () => {
-		expect(adminMutationNotice({ saved: "1" })).toEqual({
-			tone: "ok",
-			label: "Saved",
-			message: "Changes were saved successfully.",
-		});
-	});
-	it("returns null when nothing is set", () => {
-		expect(adminMutationNotice({})).toBeNull();
-	});
-	it("prioritises errors over saved flags", () => {
-		const notice = adminMutationNotice({ saved: "1", error: "x" });
-		expect(notice?.tone).toBe("danger");
-	});
-});

src/lib/admin/notice.ts

@@ -1,26 +0,0 @@
-export interface AdminMutationNotice {
-	tone: "ok" | "danger";
-	label: "Saved" | "Error";
-	message: string;
-}
-
-export function adminMutationNotice(params: {
-	saved?: string;
-	error?: string;
-}): AdminMutationNotice | null {
-	if (params.error) {
-		return {
-			tone: "danger",
-			label: "Error",
-			message: "The operation could not be completed.",
-		};
-	}
-	if (params.saved === "1") {
-		return {
-			tone: "ok",
-			label: "Saved",
-			message: "Changes were saved successfully.",
-		};
-	}
-	return null;
-}

src/lib/admin/rank-guard.test.ts

@@ -0,0 +1,82 @@
+// @ts-nocheck
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const state = vi.hoisted(() => ({
+	users: [] as Array<{ username: string; rank: number; mail: string | null }>,
+	highestRank: 8,
+}));
+
+vi.mock("@/lib/db", async () => {
+	const schema = await import("@/db/schema");
+	const { createFakeDb } = await import("@/test/fake-db");
+	const fake = createFakeDb((_table, projection) =>
+		"username" in projection ? state.users : [],
+	);
+	return { ...schema, db: fake };
+});
+
+vi.mock("@/lib/permissions", () => ({
+	getHighestRank: vi.fn(async () => state.highestRank),
+}));
+
+vi.mock("@/lib/safe-action-shared", () => ({
+	ActionError: class ActionError extends Error {},
+}));
+
+import { getHighestRank } from "@/lib/permissions";
+import { guardRank, tryGuardRank } from "./rank-guard";
+
+beforeEach(() => {
+	vi.clearAllMocks();
+	state.users = [{ username: "victim", rank: 5, mail: "[email protected]" }];
+	state.highestRank = 8;
+});
+
+describe("tryGuardRank", () => {
+	it("allows an actor at a strictly higher rank", async () => {
+		const result = await tryGuardRank(1, 7);
+		expect(result).toEqual({ ok: true, target: state.users[0] });
+	});
+
+	it("blocks an actor at the same rank", async () => {
+		state.users = [{ username: "victim", rank: 7, mail: null }];
+		const result = await tryGuardRank(1, 7);
+		expect(result).toEqual({ ok: false, reason: "rank" });
+	});
+
+	it("blocks an actor at a lower rank", async () => {
+		const result = await tryGuardRank(1, 3);
+		expect(result).toEqual({ ok: false, reason: "rank" });
+	});
+
+	it("lets the dynamic super admin act on anyone", async () => {
+		state.users = [{ username: "victim", rank: 8, mail: "[email protected]" }];
+		state.highestRank = 8;
+		const result = await tryGuardRank(1, 8);
+		expect(result).toEqual({
+			ok: true,
+			target: { username: "victim", rank: 8, mail: "[email protected]" },
+		});
+		expect(await getHighestRank()).toBe(8);
+	});
+
+	it("reports a missing target as not_found", async () => {
+		state.users = [];
+		const result = await tryGuardRank(999, 7);
+		expect(result).toEqual({ ok: false, reason: "not_found" });
+	});
+});
+
+describe("guardRank", () => {
+	it("throws a readable ActionError for a target at equal or higher rank", async () => {
+		state.users = [{ username: "victim", rank: 7, mail: null }];
+		await expect(guardRank(1, 7)).rejects.toThrow(
+			"Cannot modify user with equal or higher rank",
+		);
+	});
+
+	it("throws a readable ActionError for a missing target", async () => {
+		state.users = [];
+		await expect(guardRank(999, 7)).rejects.toThrow("User not found");
+	});
+});

src/lib/admin/rank-guard.ts

@@ -0,0 +1,65 @@
+import { eq } from "drizzle-orm";
+import { isDynamicSuperAdmin } from "@/lib/admin/authorization-policy";
+import { db, User } from "@/lib/db";
+import { getHighestRank } from "@/lib/permissions";
+import { ActionError } from "@/lib/safe-action-shared";
+
+export interface RankGuardTarget {
+	username: string;
+	rank: number;
+	mail: string | null;
+}
+
+export type RankGuardResult =
+	| { ok: true; target: RankGuardTarget }
+	| { ok: false; reason: "not_found" | "rank" };
+
+/**
+ * Refuse to act on a target whose rank is equal to or higher than the actor's,
+ * unless the actor is the dynamic super admin (the account holding the hotel's
+ * current highest rank). This is the single source of truth for the
+ * "cannot modify equal/higher rank" invariant; every admin action that mutates
+ * a specific user must go through it.
+ *
+ * Returns a discriminated result instead of throwing so callers can choose
+ * between "block the request" and "tolerate a deleted user" (e.g. lifting a
+ * ban for an account that no longer exists).
+ */
+export async function tryGuardRank(
+	targetUserId: number,
+	sessionRank: number,
+): Promise<RankGuardResult> {
+	const [target] = await db
+		.select({
+			username: User.username,
+			rank: User.rank,
+			mail: User.mail,
+		})
+		.from(User)
+		.where(eq(User.id, targetUserId))
+		.limit(1);
+	if (!target) return { ok: false, reason: "not_found" };
+	const highestRank = await getHighestRank();
+	if (
+		target.rank >= sessionRank &&
+		!isDynamicSuperAdmin(sessionRank, highestRank)
+	) {
+		return { ok: false, reason: "rank" };
+	}
+	return { ok: true, target };
+}
+
+export async function guardRank(
+	targetUserId: number,
+	sessionRank: number,
+): Promise<RankGuardTarget> {
+	const result = await tryGuardRank(targetUserId, sessionRank);
+	if (!result.ok) {
+		throw new ActionError(
+			result.reason === "not_found"
+				? "User not found"
+				: "Cannot modify user with equal or higher rank",
+		);
+	}
+	return result.target;
+}

src/lib/admin/recent-furni-resync.test.ts

@@ -1,70 +0,0 @@
-import { describe, expect, it, vi } from "vitest";
-import {
-	RECENT_FURNI_RESYNC_URL,
-	requestRecentFurniResync,
-} from "./recent-furni-resync";
-
-describe("requestRecentFurniResync", () => {
-	it("POSTs to the resync endpoint and maps the payload", async () => {
-		const fetcher = vi.fn().mockResolvedValue({
-			ok: true,
-			json: async () => ({
-				examined: 10,
-				resynced: 4,
-				failed: 1,
-				rconOk: true,
-				errors: [{ classname: "sofa", message: "boom" }],
-			}),
-		});
-		const result = await requestRecentFurniResync(fetcher);
-		expect(fetcher).toHaveBeenCalledWith(RECENT_FURNI_RESYNC_URL, {
-			method: "POST",
-		});
-		expect(result).toEqual({
-			examined: 10,
-			resynced: 4,
-			failed: 1,
-			rconOk: true,
-			errors: [{ classname: "sofa", message: "boom" }],
-		});
-	});
-
-	it("throws with the server error message on failure", async () => {
-		const fetcher = vi.fn().mockResolvedValue({
-			ok: false,
-			json: async () => ({ error: "no permission" }),
-		});
-		await expect(requestRecentFurniResync(fetcher)).rejects.toThrow(
-			"no permission",
-		);
-	});
-
-	it("falls back to a generic error message", async () => {
-		const fetcher = vi.fn().mockResolvedValue({
-			ok: false,
-			json: async () => ({}),
-		});
-		await expect(requestRecentFurniResync(fetcher)).rejects.toThrow(
-			"Furni resync failed",
-		);
-	});
-
-	it("filters malformed error entries and coerces counts", async () => {
-		const fetcher = vi.fn().mockResolvedValue({
-			ok: true,
-			json: async () => ({
-				examined: "10",
-				resynced: undefined,
-				failed: null,
-				rconOk: "yes",
-				errors: [{ classname: "a", message: "b" }, { nope: 1 }, "x"],
-			}),
-		});
-		const result = await requestRecentFurniResync(fetcher);
-		expect(result.examined).toBe(10);
-		expect(result.resynced).toBe(0);
-		expect(result.failed).toBe(0);
-		expect(result.rconOk).toBe(false);
-		expect(result.errors).toEqual([{ classname: "a", message: "b" }]);
-	});
-});

src/lib/admin/recent-furni-resync.ts

@@ -1,49 +0,0 @@
-export const RECENT_FURNI_RESYNC_URL = "/api/admin/import/furni/resync?days=7";
-
-export interface RecentFurniResyncError {
-	classname: string;
-	message: string;
-}
-
-export interface RecentFurniResyncResult {
-	examined: number;
-	resynced: number;
-	failed: number;
-	rconOk: boolean;
-	errors: RecentFurniResyncError[];
-}
-
-type AdminFetcher = (
-	input: RequestInfo | URL,
-	init?: RequestInit,
-) => Promise<Response>;
-
-export async function requestRecentFurniResync(
-	fetcher: AdminFetcher,
-): Promise<RecentFurniResyncResult> {
-	const response = await fetcher(RECENT_FURNI_RESYNC_URL, { method: "POST" });
-	const payload = (await response.json()) as Record<string, unknown>;
-	if (!response.ok) {
-		throw new Error(
-			typeof payload.error === "string" ? payload.error : "Furni resync failed",
-		);
-	}
-
-	const errors = Array.isArray(payload.errors)
-		? payload.errors.filter(
-				(value): value is RecentFurniResyncError =>
-					typeof value === "object" &&
-					value !== null &&
-					typeof (value as RecentFurniResyncError).classname === "string" &&
-					typeof (value as RecentFurniResyncError).message === "string",
-			)
-		: [];
-
-	return {
-		examined: Number(payload.examined) || 0,
-		resynced: Number(payload.resynced) || 0,
-		failed: Number(payload.failed) || 0,
-		rconOk: payload.rconOk === true,
-		errors,
-	};
-}

src/lib/import-url-safety.ts

@@ -0,0 +1,82 @@
+import { lookup } from "node:dns/promises";
+
+function isPrivateIpv4(hex: string): boolean {
+	const parts = hex.split(".").map((p) => Number(p));
+	if (parts.length !== 4 || parts.some((p) => Number.isNaN(p))) return true;
+	const [a, b] = parts;
+	if (a === 0 || a === 10) return true;
+	if (a === 127 || a === 255) return true;
+	if (a === 169 && b === 254) return true;
+	if (a === 172 && b >= 16 && b <= 31) return true;
+	if (a === 192 && b === 168) return true;
+	if (a === 100 && b >= 64 && b <= 127) return true;
+	if (a === 192 && b === 0 && parts[2] === 0 && parts[3] === 0) return true;
+	return false;
+}
+
+function isPrivateIpv6(addr: string): boolean {
+	const lower = addr.toLowerCase();
+	if (lower.startsWith("::") || lower === "::1") return true;
+	if (/^fe[89ab]/.test(lower) || /^fc|^fd/.test(lower)) return true;
+	if (lower.includes(":") && lower.split(":").length === 4) return false;
+	if (/^2001:0?db8/.test(lower)) return true;
+	return lower === "::" || lower.endsWith(":ffff:");
+}
+
+function isPrivateIp(ip: string): boolean {
+	return ip.includes(":") ? isPrivateIpv6(ip) : isPrivateIpv4(ip);
+}
+
+const BLOCKED_HOST_LABELS =
+	/^(localhost|.*\.local|.*\.localhost|.*\.internal|.*\.localdomain|.*\.lan)$/i;
+
+/**
+ * Guard a remote asset URL before it is stored or fetched. Rejects anything
+ * that could reach the host itself: non-https, embedded credentials, private
+ * IP literals, loopback/local-only hostnames and any host that resolves to a
+ * private or loopback address. Throws TypeError with a human message so admin
+ * routes can surface it as a 400.
+ */
+export async function assertSafeImportUrl(
+	url: string,
+	label: string,
+): Promise<void> {
+	const trimmed = url.trim();
+	if (!trimmed) return;
+	let parsed: URL;
+	try {
+		parsed = new URL(trimmed);
+	} catch {
+		throw new TypeError(`${label} is not a valid URL`);
+	}
+	if (parsed.protocol !== "https:") {
+		throw new TypeError(`${label} must be an https:// URL`);
+	}
+	if (parsed.username || parsed.password) {
+		throw new TypeError(`${label} must not embed credentials`);
+	}
+	const hostname = parsed.hostname.replace(/^\[|\]$/g, "");
+	if (BLOCKED_HOST_LABELS.test(hostname)) {
+		throw new TypeError(`${label} points at a local host`);
+	}
+	if (hostname) {
+		const looksLiteral =
+			hostname.includes(":") || /^\d{1,3}(\.\d{1,3}){3}$/.test(hostname);
+		if (looksLiteral) {
+			if (isPrivateIp(hostname.toLowerCase())) {
+				throw new TypeError(`${label} points at a private address`);
+			}
+			return;
+		}
+		let addresses: string[];
+		try {
+			const result = await lookup(hostname, { all: true, verbatim: true });
+			addresses = result.map((r) => r.address);
+		} catch {
+			throw new TypeError(`${label} does not resolve to a public host`);
+		}
+		if (addresses.some((ip) => isPrivateIp(ip.toLowerCase()))) {
+			throw new TypeError(`${label} resolves to a private address`);
+		}
+	}
+}

src/lib/services/admin-stats.ts

@@ -4,23 +4,27 @@ import { db } from "@/lib/db";
 import { redisCache } from "@/lib/redis-cache";
 
 /**
- * Caches admin list totals when no filters are applied (no 'where' clause),
- * preventing heavy COUNT(*) table scans on every request for large tables.
- * Filtered queries bypass cache to ensure exact search result counts.
+ * Caches admin list totals, preventing heavy COUNT(*) table scans on every
+ * request for large tables.
+ *
+ * By default only the unfiltered (global) count is cached; filtered queries
+ * hit the DB so search result counts stay exact. Callers that read a stable,
+ * fixed-filter total (e.g. "open tickets") can opt into caching it too with
+ * `{ cacheFiltered: true }` and a cacheKey unique to that filter.
  */
 export async function getCachedAdminCount(
 	table: MySqlTable,
 	cacheKey: string,
 	where?: SQL | undefined,
+	options?: { cacheFiltered?: boolean },
 ): Promise<number> {
-	// Only cache the global count (where = undefined)
-	if (where) {
+	const run = async (): Promise<number> => {
 		const [row] = await db.select({ total: count() }).from(table).where(where);
 		return Number(row?.total ?? 0);
-	}
+	};
 
-	return await redisCache(`admin:count:${cacheKey}`, 60, async () => {
-		const [row] = await db.select({ total: count() }).from(table);
-		return Number(row?.total ?? 0);
-	});
+	if (!where || options?.cacheFiltered) {
+		return await redisCache(`admin:count:${cacheKey}`, 60, run);
+	}
+	return await run();
 }

src/lib/services/catalog-items-loader.ts

@@ -127,13 +127,16 @@ export async function loadCatalogItemsData(
 	pageId: number,
 	catalog: "normal" | "bc" = "normal",
 ): Promise<CatalogItemsData> {
-	// Load items via raw query to work around pageId Int vs VARCHAR mismatch
+	// Load items via raw query to work around pageId Int vs VARCHAR mismatch.
+	// Bind the id as a *string* constant: MySQL/MariaDB coerce a constant to the
+	// column's type once, so `page_id = ?` stays sargable whether the live column
+	// is INT or VARCHAR. Casting the column (CAST(page_id AS CHAR)) would wrap it
+	// in a function and force a full table scan.
 	const pageIdStr = String(pageId);
-	// CAST: live Habbo DBs often store page_id as VARCHAR while schema maps Int.
 	const offers = catalog === "bc" ? "catalog_items_bc" : "catalog_items";
 	const rawItems = await queryRows<Record<string, unknown>>(sql`
     SELECT * FROM ${sql.raw(offers)}
-    WHERE CAST(page_id AS CHAR) = ${pageIdStr}
+    WHERE page_id = ${pageIdStr}
     ORDER BY order_number ASC, id ASC
   `);
 	// The BC table carries only a reference to the furniture: there are no price,

src/lib/services/clone-bulk-import-live.test.ts

@@ -14,7 +14,7 @@ const log = (msg: string) => appendFileSync(LOG, `${msg}\n`);
 const IMPORT_IDS = ["default-sodastudios", "default-hubbly"];
 
 describe("clone bulk import", {
-	skip: process.env.RUN_CLONE_BULK_LIVE !== "1",
+	skip: !runLive,
 }, () => {
 	beforeAll(async () => {
 		loadEnvForLiveTests();

src/lib/services/clone-feasibility-live.test.ts

@@ -11,7 +11,7 @@ const LOG = "/tmp/clone-feasibility.log";
 const log = (msg: string) => appendFileSync(LOG, `${msg}\n`);
 
 describe("clone feasibility", {
-	skip: process.env.RUN_CLONE_FEASIBILITY_LIVE !== "1",
+	skip: !runLive,
 }, () => {
 	beforeAll(async () => {
 		loadEnvForLiveTests();