From 5bac4fd719c056b4a84135789e761d5ede3a804f Mon Sep 17 00:00:00 2001 From: Hampus Date: Sun, 30 Aug 2026 22:45:29 +0200 Subject: [PATCH] perf(api): gate response schema revalidation to non-production (#2134) --- fluxer_api/src/api/Config.ts | 8 ++ .../services/interaction/MessagePinService.ts | 3 +- .../message/MessageRetrievalService.ts | 4 +- .../tests/ChannelPinsResponseShape.test.ts | 49 +++++++ fluxer_api/src/api/config/APIConfig.ts | 1 + .../api/middleware/ResponseTypeMiddleware.ts | 125 +++++++----------- .../tests/ResponseTypeMiddleware.test.ts | 52 +++++++- .../tests/OAuth2ScopeEnforcement.test.ts | 27 +++- .../api/search/MessageSearchResponseMapper.ts | 13 +- .../tests/MessageSearchEndpoint.test.ts | 41 ++++++ fluxer_api/src/api/user/UserMappers.ts | 1 - packages/config/src/MasterConfig.ts | 1 + .../src/config_loader/EnvironmentOverrides.ts | 1 + 13 files changed, 239 insertions(+), 87 deletions(-) create mode 100644 fluxer_api/src/api/channel/tests/ChannelPinsResponseShape.test.ts diff --git a/fluxer_api/src/api/Config.ts b/fluxer_api/src/api/Config.ts index 5111087ff..f69362622 100644 --- a/fluxer_api/src/api/Config.ts +++ b/fluxer_api/src/api/Config.ts @@ -65,6 +65,13 @@ function isBoolean(value: unknown): value is boolean { return typeof value === 'boolean'; } +function resolveValidateResponses(master: MasterConfig): boolean { + if (isBoolean(master.dev.validate_responses)) { + return master.dev.validate_responses; + } + return master.env !== 'production' || master.instance.self_hosted; +} + function resolveTrustClientIpHeader(proxyConfig: object): boolean { const configuredValue = Reflect.get(proxyConfig, 'trust_client_ip_header'); if (isBoolean(configuredValue)) { @@ -467,6 +474,7 @@ export function buildAPIConfigFromMaster(master: MasterConfig): APIConfig { disableRateLimits: master.dev.disable_rate_limits, testModeEnabled: master.dev.test_mode_enabled, testHarnessToken: master.dev.test_harness_token, + validateResponses: resolveValidateResponses(master), }, presignedAttachmentUploadsEnabled: master.services.api.presigned_attachment_uploads_enabled ?? false, presignedDownloadsEnabled: master.services.api.presigned_downloads_enabled ?? false, diff --git a/fluxer_api/src/api/channel/services/interaction/MessagePinService.ts b/fluxer_api/src/api/channel/services/interaction/MessagePinService.ts index de7b13f82..3b73671a9 100644 --- a/fluxer_api/src/api/channel/services/interaction/MessagePinService.ts +++ b/fluxer_api/src/api/channel/services/interaction/MessagePinService.ts @@ -109,9 +109,10 @@ export class MessagePinService extends MessageInteractionBase { const items = trimmed.flatMap((message: Message) => { const messageResponse = responseByMessageId.get(message.id.toString()); if (!messageResponse || !message.pinnedTimestamp) return []; + const {referenced_message: _referencedMessage, reactions: _reactions, ...pinnedMessage} = messageResponse; return [ { - message: messageResponse, + message: pinnedMessage, pinned_at: message.pinnedTimestamp.toISOString(), }, ]; diff --git a/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts b/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts index 91b1ea4af..9c6edcd5a 100644 --- a/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts +++ b/fluxer_api/src/api/channel/services/message/MessageRetrievalService.ts @@ -212,7 +212,9 @@ export class MessageRetrievalService { }), ), ); - const messageResponses = foundMessages.filter((message): message is MessageResponse => message !== null); + const messageResponses = foundMessages + .filter((message): message is MessageResponse => message !== null) + .map(({referenced_message: _referencedMessage, ...searchMessage}) => searchMessage); return { channels: messageResponses.length > 0 ? [await this.mapSearchChannelResponse(channel, userId, requestCache)] : [], messages: messageResponses, diff --git a/fluxer_api/src/api/channel/tests/ChannelPinsResponseShape.test.ts b/fluxer_api/src/api/channel/tests/ChannelPinsResponseShape.test.ts new file mode 100644 index 000000000..836de933f --- /dev/null +++ b/fluxer_api/src/api/channel/tests/ChannelPinsResponseShape.test.ts @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import {afterEach, beforeEach, describe, expect, test} from 'vitest'; +import {createTestAccount} from '../../auth/tests/AuthTestUtils'; +import {Config} from '../../Config'; +import {createGuild} from '../../guild/tests/GuildTestUtils'; +import {pinMessage, sendMessage} from '../../message/tests/MessageTestUtils'; +import {type ApiTestHarness, createApiTestHarness} from '../../test/ApiTestHarness'; +import {HTTP_STATUS} from '../../test/TestConstants'; +import {createBuilder} from '../../test/TestRequestBuilder'; + +describe('Channel Pins Response Shape', () => { + let harness: ApiTestHarness; + beforeEach(async () => { + harness = await createApiTestHarness(); + }); + afterEach(async () => { + Config.dev.validateResponses = true; + await harness.shutdown(); + }); + test('omits referenced_message and reactions from pinned messages when response validation is disabled', async () => { + const account = await createTestAccount(harness); + const guild = await createGuild(harness, account.token, 'Pins Shape Guild'); + const channelId = guild.system_channel_id!; + const target = await sendMessage(harness, account.token, channelId, 'pin target'); + const reply = await createBuilder<{id: string}>(harness, account.token) + .post(`/channels/${channelId}/messages`) + .body({ + content: 'pin reply', + message_reference: {channel_id: channelId, guild_id: guild.id, message_id: target.id}, + }) + .expect(HTTP_STATUS.OK) + .execute(); + await createBuilder(harness, account.token) + .put(`/channels/${channelId}/messages/${reply.id}/reactions/${encodeURIComponent('👍')}/@me`) + .body(null) + .expect(HTTP_STATUS.NO_CONTENT) + .execute(); + await pinMessage(harness, account.token, channelId, reply.id); + Config.dev.validateResponses = false; + const pins = await createBuilder<{items: Array<{message: Record}>}>(harness, account.token) + .get(`/channels/${channelId}/messages/pins`) + .expect(HTTP_STATUS.OK) + .execute(); + expect(pins.items).toHaveLength(1); + expect(pins.items[0]!.message).not.toHaveProperty('referenced_message'); + expect(pins.items[0]!.message).not.toHaveProperty('reactions'); + }); +}); diff --git a/fluxer_api/src/api/config/APIConfig.ts b/fluxer_api/src/api/config/APIConfig.ts index 10184a097..26e3d8b18 100644 --- a/fluxer_api/src/api/config/APIConfig.ts +++ b/fluxer_api/src/api/config/APIConfig.ts @@ -330,6 +330,7 @@ export interface APIConfig { disableRateLimits: boolean; testModeEnabled: boolean; testHarnessToken?: string; + validateResponses: boolean; }; presignedAttachmentUploadsEnabled: boolean; presignedDownloadsEnabled: boolean; diff --git a/fluxer_api/src/api/middleware/ResponseTypeMiddleware.ts b/fluxer_api/src/api/middleware/ResponseTypeMiddleware.ts index 5c25698d8..9593d828c 100644 --- a/fluxer_api/src/api/middleware/ResponseTypeMiddleware.ts +++ b/fluxer_api/src/api/middleware/ResponseTypeMiddleware.ts @@ -3,8 +3,9 @@ import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes'; import {InternalServerError} from '@fluxer/errors/src/domains/core/InternalServerError'; import {createLogger} from '@fluxer/logger/src/Logger'; -import type {MiddlewareHandler} from 'hono'; +import type {Context, MiddlewareHandler} from 'hono'; import type {ZodType} from 'zod'; +import {Config} from '../Config'; import type {HonoEnv} from '../types/HonoEnv'; const responseValidationLogger = createLogger('response_validation'); @@ -28,6 +29,46 @@ class ResponseValidationError extends InternalServerError { } } +async function validateAndRewriteResponse(ctx: Context, schema: ZodType): Promise { + const response = ctx.res; + const contentType = response.headers.get('content-type'); + if (!contentType?.includes('application/json')) { + return; + } + if (response.status >= 400) { + return; + } + const clonedResponse = response.clone(); + let body: unknown; + try { + body = await clonedResponse.json(); + } catch { + return; + } + const result = schema.safeParse(body); + if (!result.success) { + const validationErrors = result.error.issues.map((issue) => ({ + path: issue.path.join('.') || 'root', + message: issue.message, + })); + const errorContext = { + method: ctx.req.method, + path: ctx.req.path, + status: response.status, + validationErrors, + body, + }; + const responseValidationError = new ResponseValidationError(validationErrors); + responseValidationLogger.error(errorContext, 'Response validation failed'); + throw responseValidationError; + } + ctx.res = new Response(stringifyValidatedResponse(result.data), { + status: response.status, + statusText: response.statusText, + headers: response.headers, + }); +} + export function ResponseType( schema: T, options?: { @@ -39,49 +80,13 @@ export function ResponseType( return async (ctx, next) => { ctx.set('responseSchema' as keyof HonoEnv['Variables'], schema); await next(); - if (skipValidation) { + if (skipValidation || !Config.dev.validateResponses) { return; } - const response = ctx.res; - if (allowNoContent && response.status === 204) { + if (allowNoContent && ctx.res.status === 204) { return; } - const contentType = response.headers.get('content-type'); - if (!contentType?.includes('application/json')) { - return; - } - if (response.status >= 400) { - return; - } - const clonedResponse = response.clone(); - let body: unknown; - try { - body = await clonedResponse.json(); - } catch { - return; - } - const result = schema.safeParse(body); - if (!result.success) { - const validationErrors = result.error.issues.map((issue) => ({ - path: issue.path.join('.') || 'root', - message: issue.message, - })); - const errorContext = { - method: ctx.req.method, - path: ctx.req.path, - status: response.status, - validationErrors, - body, - }; - const responseValidationError = new ResponseValidationError(validationErrors); - responseValidationLogger.error(errorContext, 'Response validation failed'); - throw responseValidationError; - } - ctx.res = new Response(stringifyValidatedResponse(result.data), { - status: response.status, - statusText: response.statusText, - headers: response.headers, - }); + await validateAndRewriteResponse(ctx, schema); }; } @@ -207,45 +212,9 @@ export function OpenAPI( ctx.set('openapiMetadata' as keyof HonoEnv['Variables'], fullMetadata); ctx.set('responseSchema' as keyof HonoEnv['Variables'], schema); await next(); - if (!schema) { + if (!schema || !Config.dev.validateResponses) { return; } - const response = ctx.res; - const contentType = response.headers.get('content-type'); - if (!contentType?.includes('application/json')) { - return; - } - if (response.status >= 400) { - return; - } - const clonedResponse = response.clone(); - let body: unknown; - try { - body = await clonedResponse.json(); - } catch { - return; - } - const result = schema.safeParse(body); - if (!result.success) { - const validationErrors = result.error.issues.map((issue) => ({ - path: issue.path.join('.') || 'root', - message: issue.message, - })); - const errorContext = { - method: ctx.req.method, - path: ctx.req.path, - status: response.status, - validationErrors, - body, - }; - const responseValidationError = new ResponseValidationError(validationErrors); - responseValidationLogger.error(errorContext, 'Response validation failed'); - throw responseValidationError; - } - ctx.res = new Response(stringifyValidatedResponse(result.data), { - status: response.status, - statusText: response.statusText, - headers: response.headers, - }); + await validateAndRewriteResponse(ctx, schema); }; } diff --git a/fluxer_api/src/api/middleware/tests/ResponseTypeMiddleware.test.ts b/fluxer_api/src/api/middleware/tests/ResponseTypeMiddleware.test.ts index 1009f4223..bf5a72218 100644 --- a/fluxer_api/src/api/middleware/tests/ResponseTypeMiddleware.test.ts +++ b/fluxer_api/src/api/middleware/tests/ResponseTypeMiddleware.test.ts @@ -2,14 +2,23 @@ import {SnowflakeType} from '@fluxer/schema/src/primitives/SchemaPrimitives'; import {Hono} from 'hono'; -import {describe, expect, test} from 'vitest'; +import {afterAll, beforeEach, describe, expect, test} from 'vitest'; import {z} from 'zod'; +import {Config} from '../../Config'; import type {HonoEnv} from '../../types/HonoEnv'; import {OpenAPI, ResponseType} from '../ResponseTypeMiddleware'; const SnowflakeResponse = z.object({id: SnowflakeType}); describe('ResponseTypeMiddleware', () => { + beforeEach(() => { + Config.dev.validateResponses = true; + }); + + afterAll(() => { + Config.dev.validateResponses = true; + }); + test('serializes SnowflakeType response transforms as JSON strings', async () => { const app = new Hono(); app.get('/snowflake', ResponseType(SnowflakeResponse), (ctx) => ctx.json({id: '123456789012345678'})); @@ -39,4 +48,45 @@ describe('ResponseTypeMiddleware', () => { expect(response.status).toBe(200); expect(await response.json()).toEqual({id: '123456789012345678'}); }); + + test('rejects mismatching responses while validation is enabled', async () => { + const app = new Hono(); + app.get('/snowflake', ResponseType(SnowflakeResponse), (ctx) => ctx.json({id: 'not-a-snowflake'})); + + const response = await app.request('/snowflake'); + + expect(response.status).toBe(500); + }); + + test('passes the response through untouched while validation is disabled', async () => { + Config.dev.validateResponses = false; + const app = new Hono(); + app.get('/snowflake', ResponseType(SnowflakeResponse), (ctx) => ctx.json({id: 'not-a-snowflake', extra: 'kept'})); + + const response = await app.request('/snowflake'); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({id: 'not-a-snowflake', extra: 'kept'}); + }); + + test('passes the OpenAPI response through untouched while validation is disabled', async () => { + Config.dev.validateResponses = false; + const app = new Hono(); + app.get( + '/snowflake', + OpenAPI({ + operationId: 'get_snowflake_unvalidated_test', + summary: 'Get snowflake test', + description: 'Returns an unvalidated payload for response validation gating coverage.', + responseSchema: SnowflakeResponse, + tags: ['Tests'], + }), + (ctx) => ctx.json({id: 'not-a-snowflake', extra: 'kept'}), + ); + + const response = await app.request('/snowflake'); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({id: 'not-a-snowflake', extra: 'kept'}); + }); }); diff --git a/fluxer_api/src/api/oauth/tests/OAuth2ScopeEnforcement.test.ts b/fluxer_api/src/api/oauth/tests/OAuth2ScopeEnforcement.test.ts index 1dbfccdbc..1092b246f 100644 --- a/fluxer_api/src/api/oauth/tests/OAuth2ScopeEnforcement.test.ts +++ b/fluxer_api/src/api/oauth/tests/OAuth2ScopeEnforcement.test.ts @@ -1,9 +1,10 @@ // SPDX-License-Identifier: AGPL-3.0-or-later import {ADMIN_OAUTH2_APPLICATION_ID} from '@fluxer/constants/src/Core'; -import {beforeEach, describe, expect, test} from 'vitest'; +import {afterEach, beforeEach, describe, expect, test} from 'vitest'; import {createAdminApiKey} from '../../admin/tests/AdminTestUtils'; import {createTestAccount, setUserACLs} from '../../auth/tests/AuthTestUtils'; +import {Config} from '../../Config'; import {createGuild, createRole, getRoles} from '../../guild/tests/GuildTestUtils'; import {type ApiTestHarness, createApiTestHarness} from '../../test/ApiTestHarness'; import {HTTP_STATUS} from '../../test/TestConstants'; @@ -61,6 +62,9 @@ describe('OAuth2 Scope Enforcement', () => { beforeEach(async () => { harness = await createApiTestHarness(); }); + afterEach(() => { + Config.dev.validateResponses = true; + }); describe('Scope enforcement for user endpoints (/users/@me)', () => { test('GET /users/@me with bearer token succeeds when user is authenticated', async () => { const account = await createTestAccount(harness); @@ -267,6 +271,27 @@ describe('OAuth2 Scope Enforcement', () => { .expect(HTTP_STATUS.FORBIDDEN, 'MISSING_OAUTH_SCOPE') .execute(); }); + test('GET /oauth2/userinfo omits user ACLs when response validation is disabled', async () => { + const {endUser, redirectURI, application} = await createOAuth2TestSetup(harness); + const staffUser = await setUserACLs(harness, endUser, ['admin:authenticate']); + const authCodeResponse = await authorizeOAuth2(harness, staffUser.token, { + client_id: application.id, + redirect_uri: redirectURI, + scope: 'identify', + }); + const tokenResponse = await exchangeOAuth2AuthorizationCode(harness, { + client_id: application.id, + client_secret: application.client_secret, + code: authCodeResponse.code, + redirect_uri: redirectURI, + }); + Config.dev.validateResponses = false; + const json = await createBuilder>(harness, `Bearer ${tokenResponse.access_token}`) + .get('/oauth2/userinfo') + .expect(HTTP_STATUS.OK) + .execute(); + expect(json).not.toHaveProperty('acls'); + }); test('GET /oauth2/userinfo with session token is unauthorized', async () => { const account = await createTestAccount(harness); await createBuilder(harness, account.token) diff --git a/fluxer_api/src/api/search/MessageSearchResponseMapper.ts b/fluxer_api/src/api/search/MessageSearchResponseMapper.ts index e31bc98d1..fc0f8ca4b 100644 --- a/fluxer_api/src/api/search/MessageSearchResponseMapper.ts +++ b/fluxer_api/src/api/search/MessageSearchResponseMapper.ts @@ -1,6 +1,9 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -import type {MessageSearchResultsResponse} from '@fluxer/schema/src/domains/message/MessageResponseSchemas'; +import type { + MessageResponse, + MessageSearchResultsResponse, +} from '@fluxer/schema/src/domains/message/MessageResponseSchemas'; import type {UserID} from '../BrandedTypes'; import {createChannelID, createMessageID} from '../BrandedTypes'; import {mapChannelToResponse} from '../channel/ChannelMappers'; @@ -63,10 +66,12 @@ export class MessageSearchResponseMapper { }), ); const validMessageResponseEntries = messageResponsesWithEntries.filter( - (entry): entry is {message: MessageSearchResultsResponse['messages'][number]; channelId: string} => - entry !== null, + (entry): entry is {message: MessageResponse; channelId: string} => entry !== null, ); - const messageResponses = validMessageResponseEntries.map((entry) => entry.message); + const messageResponses = validMessageResponseEntries.map((entry) => { + const {referenced_message: _referencedMessage, ...searchMessage} = entry.message; + return searchMessage; + }); const orderedResponseChannelIds = new Set(validMessageResponseEntries.map((entry) => entry.channelId)); const orderedChannels = Array.from(orderedResponseChannelIds) .map((channelId) => channelById.get(channelId)) diff --git a/fluxer_api/src/api/search/tests/MessageSearchEndpoint.test.ts b/fluxer_api/src/api/search/tests/MessageSearchEndpoint.test.ts index 933290b6b..fcb2e0c20 100644 --- a/fluxer_api/src/api/search/tests/MessageSearchEndpoint.test.ts +++ b/fluxer_api/src/api/search/tests/MessageSearchEndpoint.test.ts @@ -3,6 +3,7 @@ import {afterEach, beforeEach, describe, expect, test} from 'vitest'; import {createTestAccount} from '../../auth/tests/AuthTestUtils'; import {createChannelID, createMessageID} from '../../BrandedTypes'; +import {Config} from '../../Config'; import {ChannelRepository} from '../../channel/ChannelRepository'; import {createGuild} from '../../guild/tests/GuildTestUtils'; import { @@ -48,8 +49,48 @@ describe('Message Search Endpoint', () => { harness = await createApiTestHarness({search: 'enabled'}); }); afterEach(async () => { + Config.dev.validateResponses = true; await harness.shutdown(); }); + describe('Response Shape', () => { + test('omits referenced_message from search hits when response validation is disabled', async () => { + const account = await createTestAccount(harness); + const guild = await createGuild(harness, account.token, 'Reply Search Guild'); + const channelId = guild.system_channel_id!; + const uniqueContent = `reply-search-term-${Date.now()}`; + const target = await sendMessage(harness, account.token, channelId, `${uniqueContent} target`); + await createBuilder(harness, account.token) + .post(`/channels/${channelId}/messages`) + .body({ + content: `${uniqueContent} reply`, + message_reference: {channel_id: channelId, guild_id: guild.id, message_id: target.id}, + }) + .expect(HTTP_STATUS.OK) + .execute(); + await markChannelAsIndexed(harness, channelId); + Config.dev.validateResponses = false; + let hits: MessageSearchResult['messages'] = []; + for (let attempt = 0; attempt < 20; attempt++) { + const result = await createBuilder(harness, account.token) + .post('/search/messages') + .body({ + content: uniqueContent, + context_channel_id: channelId, + }) + .expect(HTTP_STATUS.OK) + .execute(); + if (isSearchResult(result) && result.messages.length > 0) { + hits = result.messages; + break; + } + await wait(TEST_TIMEOUTS.QUICK); + } + expect(hits.length).toBeGreaterThan(0); + for (const message of hits) { + expect(message).not.toHaveProperty('referenced_message'); + } + }); + }); describe('Authentication', () => { test('requires authentication (401 without token)', async () => { await createBuilder(harness, '') diff --git a/fluxer_api/src/api/user/UserMappers.ts b/fluxer_api/src/api/user/UserMappers.ts index cebd6d0ea..9a3f04b1a 100644 --- a/fluxer_api/src/api/user/UserMappers.ts +++ b/fluxer_api/src/api/user/UserMappers.ts @@ -234,7 +234,6 @@ export function mapUserToOAuthResponse( global_name: user.globalName ?? null, bot: user.isBot || false, system: user.isSystem || false, - acls: Array.from(user.acls), avatar_color: user.avatarColor, }; } diff --git a/packages/config/src/MasterConfig.ts b/packages/config/src/MasterConfig.ts index 7a3fe40de..031a94f8f 100644 --- a/packages/config/src/MasterConfig.ts +++ b/packages/config/src/MasterConfig.ts @@ -366,6 +366,7 @@ export interface MasterConfig { disable_rate_limits: boolean; test_mode_enabled: boolean; test_harness_token?: string; + validate_responses?: boolean; }; geoip: { maxmind_db_path: string; diff --git a/packages/config/src/config_loader/EnvironmentOverrides.ts b/packages/config/src/config_loader/EnvironmentOverrides.ts index de0e3370b..a3f4ccc60 100644 --- a/packages/config/src/config_loader/EnvironmentOverrides.ts +++ b/packages/config/src/config_loader/EnvironmentOverrides.ts @@ -411,6 +411,7 @@ const NAMED_FLUXER_ENV_OVERRIDES: Record = { FLUXER_DISABLE_RATE_LIMITS: {path: ['dev', 'disable_rate_limits'], parse: parseEnvValue}, FLUXER_TEST_MODE_ENABLED: {path: ['dev', 'test_mode_enabled'], parse: parseEnvValue}, FLUXER_TEST_HARNESS_TOKEN: {path: ['dev', 'test_harness_token']}, + FLUXER_VALIDATE_RESPONSES: {path: ['dev', 'validate_responses'], parse: parseEnvValue}, FLUXER_GEOIP_DB_PATH: {path: ['geoip', 'maxmind_db_path']}, };