fix(housekeeping): harden preference persistence

src/actions/housekeeping-preferences.test.ts

@@ -1,89 +1,90 @@
-import { describe, expect, it, vi } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
 
 vi.mock("@/features/housekeeping/foundation/server-capability-context", () => ({
 	getHousekeepingCapabilityContext: vi.fn(),
 }));
 
-import type { HousekeepingPreferencesRepository } from "@/features/housekeeping/foundation/preferences/repository";
+const preferenceRepository = vi.hoisted(() => ({
+	read: vi.fn(),
+	upsert: vi.fn(),
+}));
+
+vi.mock("@/lib/housekeeping-preferences-repository", () => ({
+	housekeepingPreferencesRepository: preferenceRepository,
+}));
+
 import { defaultHousekeepingPreferences } from "@/features/housekeeping/foundation/preferences/schema";
-import type { HousekeepingRegistry } from "@/features/housekeeping/foundation/registry";
+import { getHousekeepingCapabilityContext } from "@/features/housekeeping/foundation/server-capability-context";
 import {
-	type HousekeepingPreferencesActionDependencies,
 	loadHousekeepingPreferences,
 	saveHousekeepingPreferences,
 } from "./housekeeping-preferences";
 
-const registry: HousekeepingRegistry = { domains: [] };
 const allowedContext = {
 	actor: { id: 42, username: "operator", rank: 0 },
 	isSuperAdmin: false,
-	has: (slug: string) => slug === "admin.dashboard",
-	hasAny: (...slugs: string[]) => slugs.includes("admin.dashboard"),
-	hasAll: (...slugs: string[]) =>
-		slugs.every((slug) => slug === "admin.dashboard"),
+	has: () => true,
+	hasAny: () => true,
+	hasAll: () => true,
 };
 
-function dependencies(
-	context = allowedContext,
-): HousekeepingPreferencesActionDependencies & {
-	repository: HousekeepingPreferencesRepository;
-} {
-	return {
-		repository: {
-			read: vi.fn().mockResolvedValue(defaultHousekeepingPreferences()),
-			upsert: vi.fn(),
-		},
-		registry,
-		getContext: vi.fn().mockResolvedValue(context),
-	};
-}
+beforeEach(() => {
+	vi.clearAllMocks();
+	vi.mocked(getHousekeepingCapabilityContext).mockResolvedValue(allowedContext);
+	preferenceRepository.read.mockResolvedValue(defaultHousekeepingPreferences());
+});
 
 describe("housekeeping preference actions", () => {
-	it("derives the read owner from request capability context", async () => {
-		const deps = dependencies();
-
-		const result = await loadHousekeepingPreferences(deps);
-
-		expect(result.ok).toBe(true);
-		expect(deps.repository.read).toHaveBeenCalledWith(42);
-		expect(deps.getContext).toHaveBeenCalledOnce();
+	it("does not expose dependency or user identity parameters to callers", () => {
+		expect(loadHousekeepingPreferences).toHaveLength(0);
+		expect(saveHousekeepingPreferences).toHaveLength(1);
 	});
 
-	it("rejects a denied operator without reading or writing another user preferences", async () => {
-		const deps = dependencies({ ...allowedContext, hasAny: () => false });
+	it("derives the read owner from the server capability context", async () => {
+		const result = await loadHousekeepingPreferences();
+
+		expect(result.ok).toBe(true);
+		expect(preferenceRepository.read).toHaveBeenCalledWith(42);
+	});
+
+	it("rejects a denied operator without reading or writing preferences", async () => {
+		vi.mocked(getHousekeepingCapabilityContext).mockResolvedValueOnce({
+			...allowedContext,
+			hasAny: () => false,
+		});
 
 		const result = await saveHousekeepingPreferences(
 			defaultHousekeepingPreferences(),
-			deps,
 		);
 
 		expect(result).toMatchObject({ ok: false, error: { code: "FORBIDDEN" } });
-		expect(deps.repository.read).not.toHaveBeenCalled();
-		expect(deps.repository.upsert).not.toHaveBeenCalled();
+		expect(preferenceRepository.read).not.toHaveBeenCalled();
+		expect(preferenceRepository.upsert).not.toHaveBeenCalled();
 	});
 
-	it("validates writes and persists them for the context actor only", async () => {
-		const deps = dependencies();
+	it("reconciles input before persisting or returning it", async () => {
 		const value = {
 			...defaultHousekeepingPreferences(),
-			pinnedRouteIds: ["people.users"],
+			pinnedRouteIds: ["removed.route"],
+			pinnedCommandIds: ["removed.command"],
 		};
 
-		const result = await saveHousekeepingPreferences(value, deps);
+		const result = await saveHousekeepingPreferences(value);
 
-		expect(result).toMatchObject({ ok: true, data: value });
-		expect(deps.repository.upsert).toHaveBeenCalledWith(42, value);
+		expect(result).toMatchObject({
+			ok: true,
+			data: { pinnedRouteIds: [], pinnedCommandIds: [] },
+		});
+		expect(preferenceRepository.upsert).toHaveBeenCalledWith(
+			42,
+			expect.objectContaining({ pinnedRouteIds: [], pinnedCommandIds: [] }),
+		);
 	});
 
 	it("returns a validation result before an invalid payload reaches persistence", async () => {
-		const deps = dependencies();
-
-		const result = await saveHousekeepingPreferences(
-			{ schemaVersion: 2 },
-			deps,
-		);
+		const result = await saveHousekeepingPreferences({ schemaVersion: 2 });
 
 		expect(result).toMatchObject({ ok: false, error: { code: "VALIDATION" } });
-		expect(deps.repository.upsert).not.toHaveBeenCalled();
+		expect(preferenceRepository.upsert).not.toHaveBeenCalled();
 	});
 });

src/actions/housekeeping-preferences.ts

@@ -1,42 +1,41 @@
+"use server";
+
 import { authorizeHousekeeping } from "@/features/housekeeping/foundation/authorization";
 import {
 	anyCapability,
 	fail,
-	type HousekeepingCapabilityContext,
 	type HousekeepingResult,
 	ok,
 } from "@/features/housekeeping/foundation/contracts";
 import { createCorrelationId } from "@/features/housekeeping/foundation/correlation";
 import { reconcilePreferences } from "@/features/housekeeping/foundation/preferences/reconcile";
-import type { HousekeepingPreferencesRepository } from "@/features/housekeeping/foundation/preferences/repository";
 import {
 	type HousekeepingPreferences,
 	housekeepingPreferencesSchema,
 } from "@/features/housekeeping/foundation/preferences/schema";
-import type { HousekeepingRegistry } from "@/features/housekeeping/foundation/registry";
+import { createHousekeepingRegistry } from "@/features/housekeeping/foundation/registry";
 import { getHousekeepingCapabilityContext } from "@/features/housekeeping/foundation/server-capability-context";
+import { HOUSEKEEPING_MANIFESTS } from "@/features/housekeeping/manifests";
+import { housekeepingPreferencesRepository } from "@/lib/housekeeping-preferences-repository";
 import { PERMS } from "@/lib/permission-slugs";
 
 const preferencesCapability = anyCapability(PERMS.ADMIN_DASHBOARD);
+const housekeepingRegistry = createHousekeepingRegistry(HOUSEKEEPING_MANIFESTS);
 
-export interface HousekeepingPreferencesActionDependencies {
-	repository: HousekeepingPreferencesRepository;
-	registry: HousekeepingRegistry;
-	getContext?: () => Promise<HousekeepingCapabilityContext>;
-}
-
-export async function loadHousekeepingPreferences(
-	dependencies: HousekeepingPreferencesActionDependencies,
-): Promise<HousekeepingResult<HousekeepingPreferences>> {
-	const context = await resolveContext(dependencies);
+export async function loadHousekeepingPreferences(): Promise<
+	HousekeepingResult<HousekeepingPreferences>
+> {
+	const context = await getHousekeepingCapabilityContext();
 	const authorization = authorizeHousekeeping(context, preferencesCapability);
 	if (!authorization.ok) return authorization;
 
 	const correlationId = createCorrelationId();
 	try {
-		const stored = await dependencies.repository.read(context.actor.id);
+		const stored = await housekeepingPreferencesRepository.read(
+			context.actor.id,
+		);
 		return ok(
-			reconcilePreferences(stored, dependencies.registry, context),
+			reconcilePreferences(stored, housekeepingRegistry, context),
 			correlationId,
 		);
 	} catch {
@@ -50,9 +49,8 @@ export async function loadHousekeepingPreferences(
 
 export async function saveHousekeepingPreferences(
 	input: unknown,
-	dependencies: HousekeepingPreferencesActionDependencies,
 ): Promise<HousekeepingResult<HousekeepingPreferences>> {
-	const context = await resolveContext(dependencies);
+	const context = await getHousekeepingCapabilityContext();
 	const authorization = authorizeHousekeeping(context, preferencesCapability);
 	if (!authorization.ok) return authorization;
 
@@ -66,9 +64,17 @@ export async function saveHousekeepingPreferences(
 		);
 	}
 
+	const reconciled = reconcilePreferences(
+		parsed.data,
+		housekeepingRegistry,
+		context,
+	);
 	try {
-		await dependencies.repository.upsert(context.actor.id, parsed.data);
-		return ok(parsed.data, correlationId);
+		await housekeepingPreferencesRepository.upsert(
+			context.actor.id,
+			reconciled,
+		);
+		return ok(reconciled, correlationId);
 	} catch {
 		return fail(
 			"INTERNAL",
@@ -77,11 +83,3 @@ export async function saveHousekeepingPreferences(
 		);
 	}
 }
-
-async function resolveContext(
-	dependencies: HousekeepingPreferencesActionDependencies,
-): Promise<HousekeepingCapabilityContext> {
-	return dependencies.getContext
-		? dependencies.getContext()
-		: getHousekeepingCapabilityContext();
-}

src/features/housekeeping/foundation/preferences/repository.test.ts

@@ -1,4 +1,16 @@
 import { describe, expect, it, vi } from "vitest";
+
+const drizzleMocks = vi.hoisted(() => ({
+	eq: vi.fn((column: unknown, value: unknown) => ({ column, value })),
+}));
+
+vi.mock("drizzle-orm", async (importOriginal) => ({
+	...(await importOriginal<typeof import("drizzle-orm")>()),
+	eq: drizzleMocks.eq,
+}));
+
+import { HousekeepingUserPreferences } from "@/lib/db";
+import { createDrizzleHousekeepingPreferencesStorage } from "@/lib/housekeeping-preferences-repository";
 import {
 	createHousekeepingPreferencesRepository,
 	type HousekeepingPreferencesStorage,
@@ -39,4 +51,90 @@ describe("housekeeping preferences repository", () => {
 			payload: JSON.stringify(value),
 		});
 	});
+
+	it("returns a fresh default for malformed or incompatible stored values", async () => {
+		const storage: HousekeepingPreferencesStorage = {
+			findByUserId: vi
+				.fn()
+				.mockResolvedValueOnce({ schemaVersion: 1, payload: "{broken" })
+				.mockResolvedValueOnce({ schemaVersion: 2, payload: "{}" })
+				.mockResolvedValueOnce({
+					schemaVersion: 1,
+					payload: JSON.stringify({ schemaVersion: 1 }),
+				}),
+			upsert: vi.fn(),
+		};
+		const repository = createHousekeepingPreferencesRepository(storage);
+
+		await expect(repository.read(42)).resolves.toEqual(
+			defaultHousekeepingPreferences(),
+		);
+		await expect(repository.read(42)).resolves.toEqual(
+			defaultHousekeepingPreferences(),
+		);
+		await expect(repository.read(42)).resolves.toEqual(
+			defaultHousekeepingPreferences(),
+		);
+	});
+
+	it("preserves genuine storage failures instead of treating them as corrupt preferences", async () => {
+		const storage: HousekeepingPreferencesStorage = {
+			findByUserId: vi
+				.fn()
+				.mockRejectedValue(new Error("database unavailable")),
+			upsert: vi.fn(),
+		};
+
+		await expect(
+			createHousekeepingPreferencesRepository(storage).read(42),
+		).rejects.toThrow("database unavailable");
+	});
+
+	it("uses the preferences table and user ID for the typed Drizzle read and upsert", async () => {
+		const limit = vi.fn().mockResolvedValue([]);
+		const where = vi.fn().mockReturnValue({ limit });
+		const from = vi.fn().mockReturnValue({ where });
+		const select = vi.fn().mockReturnValue({ from });
+		const onDuplicateKeyUpdate = vi.fn().mockResolvedValue(undefined);
+		const values = vi.fn().mockReturnValue({ onDuplicateKeyUpdate });
+		const insert = vi.fn().mockReturnValue({ values });
+		const storage = createDrizzleHousekeepingPreferencesStorage({
+			select,
+			insert,
+		} as never);
+		const value = defaultHousekeepingPreferences();
+
+		await storage.findByUserId(42);
+		await storage.upsert({
+			userId: 42,
+			schemaVersion: 1,
+			payload: JSON.stringify(value),
+		});
+
+		expect(select).toHaveBeenCalledWith({
+			schemaVersion: HousekeepingUserPreferences.schemaVersion,
+			payload: HousekeepingUserPreferences.payload,
+		});
+		expect(from).toHaveBeenCalledWith(HousekeepingUserPreferences);
+		expect(drizzleMocks.eq).toHaveBeenCalledWith(
+			HousekeepingUserPreferences.userId,
+			42,
+		);
+		expect(where).toHaveBeenCalledWith({
+			column: HousekeepingUserPreferences.userId,
+			value: 42,
+		});
+		expect(values).toHaveBeenCalledWith({
+			userId: 42,
+			schemaVersion: 1,
+			payload: JSON.stringify(value),
+		});
+		expect(onDuplicateKeyUpdate).toHaveBeenCalledWith({
+			set: expect.objectContaining({
+				schemaVersion: 1,
+				payload: JSON.stringify(value),
+				updatedAt: expect.any(Date),
+			}),
+		});
+	});
 });

src/features/housekeeping/foundation/preferences/repository.ts

@@ -32,11 +32,15 @@ export function createHousekeepingPreferencesRepository(
 		async read(userId) {
 			const row = await storage.findByUserId(userId);
 			if (!row) return defaultHousekeepingPreferences();
-			if (row.schemaVersion !== 1) {
-				throw new Error("unsupported housekeeping preferences schema version");
-			}
 
-			return parseHousekeepingPreferences(row.payload);
+			try {
+				if (row.schemaVersion !== 1) {
+					return defaultHousekeepingPreferences();
+				}
+				return parseHousekeepingPreferences(row.payload);
+			} catch {
+				return defaultHousekeepingPreferences();
+			}
 		},
 		async upsert(userId, value) {
 			await storage.upsert({

src/features/housekeeping/foundation/preferences/schema.test.ts

@@ -43,4 +43,32 @@ describe("housekeeping preferences schema", () => {
 
 		expect(parseHousekeepingPreferences(JSON.stringify(value))).toEqual(value);
 	});
+
+	it("rejects identifiers and lists that exceed the bounded presentation payload", () => {
+		const value = defaultHousekeepingPreferences();
+
+		expect(
+			housekeepingPreferencesSchema.safeParse({
+				...value,
+				pinnedRouteIds: ["a".repeat(129)],
+			}).success,
+		).toBe(false);
+		expect(
+			housekeepingPreferencesSchema.safeParse({
+				...value,
+				pinnedRouteIds: Array.from(
+					{ length: 65 },
+					(_, index) => `route.${index}`,
+				),
+			}).success,
+		).toBe(false);
+		expect(
+			housekeepingPreferencesSchema.safeParse({
+				...value,
+				pinnedRouteIds: Array.from({ length: 33 }, (_, index) =>
+					`${index}`.padEnd(128, "a"),
+				),
+			}).success,
+		).toBe(false);
+	});
 });

src/features/housekeeping/foundation/preferences/schema.ts

@@ -1,5 +1,9 @@
 import { z } from "zod";
 
+export const HOUSEKEEPING_PREFERENCE_MAX_IDENTIFIER_LENGTH = 128;
+export const HOUSEKEEPING_PREFERENCE_MAX_ITEMS_PER_ARRAY = 64;
+export const HOUSEKEEPING_PREFERENCE_MAX_SERIALIZED_BYTES = 4096;
+
 export interface HousekeepingPreferences {
 	schemaVersion: 1;
 	pinnedRouteIds: string[];
@@ -10,7 +14,10 @@ export interface HousekeepingPreferences {
 }
 
 const uniqueIdentifiers = z
-	.array(z.string().trim().min(1))
+	.array(
+		z.string().trim().min(1).max(HOUSEKEEPING_PREFERENCE_MAX_IDENTIFIER_LENGTH),
+	)
+	.max(HOUSEKEEPING_PREFERENCE_MAX_ITEMS_PER_ARRAY)
 	.superRefine((values, context) => {
 		const seen = new Set<string>();
 		for (const [index, value] of values.entries()) {
@@ -35,7 +42,18 @@ export const housekeepingPreferencesSchema: z.ZodType<HousekeepingPreferences> =
 			widgetOrder: uniqueIdentifiers,
 			enabledOptionalWidgetIds: uniqueIdentifiers,
 		})
-		.strict();
+		.strict()
+		.superRefine((preferences, context) => {
+			if (
+				JSON.stringify(preferences).length >
+				HOUSEKEEPING_PREFERENCE_MAX_SERIALIZED_BYTES
+			) {
+				context.addIssue({
+					code: "custom",
+					message: "housekeeping preferences payload is too large",
+				});
+			}
+		});
 
 export function defaultHousekeepingPreferences(): HousekeepingPreferences {
 	return {

src/lib/housekeeping-preferences-repository.ts

@@ -0,0 +1,43 @@
+import { eq } from "drizzle-orm";
+import {
+	createHousekeepingPreferencesRepository,
+	type HousekeepingPreferencesStorage,
+} from "@/features/housekeeping/foundation/preferences/repository";
+import { type Db, db, HousekeepingUserPreferences } from "@/lib/db";
+
+type HousekeepingPreferencesDb = Pick<Db, "select" | "insert">;
+
+export function createDrizzleHousekeepingPreferencesStorage(
+	database: HousekeepingPreferencesDb,
+): HousekeepingPreferencesStorage {
+	return {
+		async findByUserId(userId) {
+			const rows = await database
+				.select({
+					schemaVersion: HousekeepingUserPreferences.schemaVersion,
+					payload: HousekeepingUserPreferences.payload,
+				})
+				.from(HousekeepingUserPreferences)
+				.where(eq(HousekeepingUserPreferences.userId, userId))
+				.limit(1);
+			return rows[0] ?? null;
+		},
+		async upsert(row) {
+			await database
+				.insert(HousekeepingUserPreferences)
+				.values(row)
+				.onDuplicateKeyUpdate({
+					set: {
+						schemaVersion: row.schemaVersion,
+						payload: row.payload,
+						updatedAt: new Date(),
+					},
+				});
+		},
+	};
+}
+
+export const housekeepingPreferencesRepository =
+	createHousekeepingPreferencesRepository(
+		createDrizzleHousekeepingPreferencesStorage(db),
+	);