diff --git a/fluxer_api/src/api/admin/tests/LimitConfigRemovedKeys.test.ts b/fluxer_api/src/api/admin/tests/LimitConfigRemovedKeys.test.ts new file mode 100644 index 000000000..b9c19772d --- /dev/null +++ b/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; + +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 { + return await setUserACLs(harness, await createTestAccount(harness), [ + AdminACLs.AUTHENTICATE, + AdminACLs.INSTANCE_LIMIT_CONFIG_VIEW, + AdminACLs.INSTANCE_LIMIT_CONFIG_UPDATE, + ]); + } + + async function storeRuleFromOlderRelease(): Promise { + 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(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(harness, admin.token) + .get('/admin/limit-config') + .execute(); + await createBuilder(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(harness, admin.token) + .get('/admin/limit-config') + .execute(); + expect(read.rules.some((rule) => rule.id === 'packs')).toBe(false); + await createBuilder(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(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, + }), + ]); + }); +}); diff --git a/fluxer_api/src/api/constants/LimitConfig.ts b/fluxer_api/src/api/constants/LimitConfig.ts index 1b14459e0..0c527d23b 100644 --- a/fluxer_api/src/api/constants/LimitConfig.ts +++ b/fluxer_api/src/api/constants/LimitConfig.ts @@ -103,7 +103,7 @@ export function mergeWithCurrentDefaults( const mergedRules: Array = []; const existingRulesMap = new Map(); 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>): Partial> { - const clamped = {...limits}; +function readKnownLimits(limits: Partial>): Partial> { + const known: Partial> = {}; 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( diff --git a/packages/schema/src/domains/admin/AdminSchemas.ts b/packages/schema/src/domains/admin/AdminSchemas.ts index 2f1cae86b..ca8cadad6 100644 --- a/packages/schema/src/domains/admin/AdminSchemas.ts +++ b/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).includes(key)); - }, - {error: 'Invalid limit key detected'}, - ) .superRefine((limits, ctx) => { + for (const key of Object.keys(limits)) { + if (!(LIMIT_KEYS as ReadonlyArray).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];