fix(limits): drop removed limit keys from stored rules (#3520)

fluxer_api/src/api/admin/tests/LimitConfigRemovedKeys.test.ts

@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import type {TestAccount} from '@app/api/auth/tests/AuthTestUtils';
+import {createTestAccount, setUserACLs} from '@app/api/auth/tests/AuthTestUtils';
+import {getConfig} from '@app/api/Config';
+import {getInstanceConfigRepository, getLimitConfigService} from '@app/api/middleware/ServiceSingletons';
+import {type ApiTestHarness, createApiTestHarness} from '@app/api/test/ApiTestHarness';
+import {HTTP_STATUS} from '@app/api/test/TestConstants';
+import {createBuilder} from '@app/api/test/TestRequestBuilder';
+import {AdminACLs} from '@fluxer/constants/src/AdminACLs';
+import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes';
+import {ValidationErrorCodes} from '@fluxer/constants/src/ValidationErrorCodes';
+import type {LimitRule} from '@fluxer/limits/src/LimitTypes';
+import type {LimitConfigGetResponse} from '@fluxer/schema/src/domains/admin/AdminSchemas';
+import {afterAll, beforeAll, beforeEach, describe, expect, it} from 'vitest';
+import type {z} from 'zod';
+
+type LimitConfigResponse = z.infer<typeof LimitConfigGetResponse>;
+
+const REMOVED_KEY = 'max_created_packs';
+
+describe.each([true, false])('limit config with a key from an older release (self-hosted: %s)', (selfHosted) => {
+	let harness: ApiTestHarness;
+	let originalSelfHosted: boolean;
+
+	beforeAll(async () => {
+		originalSelfHosted = getConfig().instance.selfHosted;
+		getConfig().instance.selfHosted = selfHosted;
+		harness = await createApiTestHarness();
+	});
+
+	beforeEach(async () => {
+		await harness.reset();
+		await getLimitConfigService().refreshCache();
+	});
+
+	afterAll(async () => {
+		await harness.shutdown();
+		getConfig().instance.selfHosted = originalSelfHosted;
+	});
+
+	async function createAdmin(): Promise<TestAccount> {
+		return await setUserACLs(harness, await createTestAccount(harness), [
+			AdminACLs.AUTHENTICATE,
+			AdminACLs.INSTANCE_LIMIT_CONFIG_VIEW,
+			AdminACLs.INSTANCE_LIMIT_CONFIG_UPDATE,
+		]);
+	}
+
+	async function storeRuleFromOlderRelease(): Promise<void> {
+		const config = getLimitConfigService().getConfigSnapshot();
+		const legacyRule = {
+			id: 'vip',
+			filters: {traits: ['vip']},
+			limits: {max_bio_length: 200, [REMOVED_KEY]: 5},
+		} as LimitRule;
+		await getInstanceConfigRepository().setLimitConfig({
+			traitDefinitions: [...config.traitDefinitions, 'vip'],
+			rules: [...config.rules, legacyRule],
+		});
+		await getLimitConfigService().refreshCache();
+	}
+
+	it('leaves the removed key out when reading the stored config', async () => {
+		const admin = await createAdmin();
+		await storeRuleFromOlderRelease();
+		const response = await createBuilder<LimitConfigResponse>(harness, admin.token)
+			.get('/admin/limit-config')
+			.execute();
+		const vip = response.limit_config.rules.find((rule) => rule.id === 'vip');
+		expect(vip?.limits).toEqual({max_bio_length: 200});
+	});
+
+	it('saves the config it read back and drops the removed key from storage', async () => {
+		const admin = await createAdmin();
+		await storeRuleFromOlderRelease();
+		const {limit_config: config} = await createBuilder<LimitConfigResponse>(harness, admin.token)
+			.get('/admin/limit-config')
+			.execute();
+		await createBuilder<LimitConfigResponse>(harness, admin.token)
+			.put('/admin/limit-config')
+			.body({
+				limit_config: {
+					traitDefinitions: config.traitDefinitions,
+					rules: config.rules.map(({modifiedFields: _modifiedFields, ...rule}) => rule),
+				},
+			})
+			.execute();
+		const {config: stored} = await getInstanceConfigRepository().readLimitConfigInputs();
+		const vip = stored?.rules.find((rule) => rule.id === 'vip');
+		expect(vip?.limits).toEqual({max_bio_length: 200});
+	});
+
+	it('drops a rule whose only limits were removed keys', async () => {
+		const admin = await createAdmin();
+		const config = getLimitConfigService().getConfigSnapshot();
+		await getInstanceConfigRepository().setLimitConfig({
+			traitDefinitions: [...config.traitDefinitions, 'packs'],
+			rules: [...config.rules, {id: 'packs', filters: {traits: ['packs']}, limits: {[REMOVED_KEY]: 5}} as LimitRule],
+		});
+		await getLimitConfigService().refreshCache();
+		const {limit_config: read} = await createBuilder<LimitConfigResponse>(harness, admin.token)
+			.get('/admin/limit-config')
+			.execute();
+		expect(read.rules.some((rule) => rule.id === 'packs')).toBe(false);
+		await createBuilder<LimitConfigResponse>(harness, admin.token)
+			.put('/admin/limit-config')
+			.body({
+				limit_config: {
+					traitDefinitions: read.traitDefinitions,
+					rules: read.rules.map(({modifiedFields: _modifiedFields, ...rule}) => rule),
+				},
+			})
+			.execute();
+	});
+
+	it('names the unknown key in the validation error', async () => {
+		const admin = await createAdmin();
+		const {limit_config: config} = await createBuilder<LimitConfigResponse>(harness, admin.token)
+			.get('/admin/limit-config')
+			.execute();
+		const response = await createBuilder<{errors: Array<{path: string; code: string}>}>(harness, admin.token)
+			.put('/admin/limit-config')
+			.body({
+				limit_config: {
+					traitDefinitions: [...config.traitDefinitions, 'vip'],
+					rules: [
+						...config.rules.map(({modifiedFields: _modifiedFields, ...rule}) => rule),
+						{id: 'vip', filters: {traits: ['vip']}, limits: {max_bio_length: 200, [REMOVED_KEY]: 5}},
+					],
+				},
+			})
+			.expect(HTTP_STATUS.BAD_REQUEST, APIErrorCodes.INVALID_FORM_BODY)
+			.execute();
+		expect(response.errors).toEqual([
+			expect.objectContaining({
+				path: `limit_config.rules.${config.rules.length}.limits.${REMOVED_KEY}`,
+				code: ValidationErrorCodes.INVALID_FORMAT,
+			}),
+		]);
+	});
+});

fluxer_api/src/api/constants/LimitConfig.ts

@@ -103,7 +103,7 @@ export function mergeWithCurrentDefaults(
 	const mergedRules: Array<LimitRule> = [];
 	const existingRulesMap = new Map<string, LimitRule>();
 	for (const rule of stored.rules) {
-		existingRulesMap.set(rule.id, {...rule, limits: clampToAllowedRange(rule.limits)});
+		existingRulesMap.set(rule.id, {...rule, limits: readKnownLimits(rule.limits)});
 	}
 	for (const defaultRule of newDefaults.rules) {
 		const existingRule = existingRulesMap.get(defaultRule.id);
@@ -125,6 +125,9 @@ export function mergeWithCurrentDefaults(
 		existingRulesMap.delete(defaultRule.id);
 	}
 	for (const [, rule] of existingRulesMap) {
+		if (Object.keys(rule.limits).length === 0) {
+			continue;
+		}
 		const modifiedFields = findModifiedLimits(rule.limits, defaultBaseRule.limits);
 		mergedRules.push({
 			id: rule.id,
@@ -231,16 +234,17 @@ function wasSavedUnderOtherDefaults(
 	);
 }
 
-function clampToAllowedRange(limits: Partial<Record<LimitKey, number>>): Partial<Record<LimitKey, number>> {
-	const clamped = {...limits};
+function readKnownLimits(limits: Partial<Record<LimitKey, number>>): Partial<Record<LimitKey, number>> {
+	const known: Partial<Record<LimitKey, number>> = {};
 	for (const key of LIMIT_KEYS) {
-		const value = clamped[key];
-		const {min = 0, max} = LIMIT_KEY_METADATA[key];
-		if (value !== undefined && max !== undefined) {
-			clamped[key] = Math.min(Math.max(value, min), max);
+		const value = limits[key];
+		if (value === undefined) {
+			continue;
 		}
+		const {min = 0, max} = LIMIT_KEY_METADATA[key];
+		known[key] = max === undefined ? value : Math.min(Math.max(value, min), max);
 	}
-	return clamped;
+	return known;
 }
 
 function pickModifiedLimits(

packages/schema/src/domains/admin/AdminSchemas.ts

@@ -951,14 +951,12 @@ const LimitRuleSchema = z.object({
 	filters: LimitFilterSchema.optional().describe('Optional filters that scope the rule'),
 	limits: z
 		.record(z.string(), NonNegativeSafeIntegerType)
-		.refine(
-			(limits) => {
-				const limitKeys = Object.keys(limits);
-				return limitKeys.every((key) => (LIMIT_KEYS as ReadonlyArray<string>).includes(key));
-			},
-			{error: 'Invalid limit key detected'},
-		)
 		.superRefine((limits, ctx) => {
+			for (const key of Object.keys(limits)) {
+				if (!(LIMIT_KEYS as ReadonlyArray<string>).includes(key)) {
+					ctx.addIssue({code: 'custom', message: ValidationErrorCodes.INVALID_FORMAT, path: [key]});
+				}
+			}
 			for (const key of LIMIT_KEYS) {
 				const value = limits[key];
 				const {min = 0, max} = LIMIT_KEY_METADATA[key];