mirror of
https://github.com/fluxerapp/fluxer
synced 2026-10-17 21:15:32 +09:00
perf(api): gate response schema revalidation to non-production (#2134)
This commit is contained in:
1 parent
dd610e4c0f
commit
5bac4fd719
13 files changed
+239
-87
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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(),
|
||||
},
|
||||
];
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, unknown>}>}>(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');
|
||||
});
|
||||
});
|
||||
@@ -330,6 +330,7 @@ export interface APIConfig {
|
||||
disableRateLimits: boolean;
|
||||
testModeEnabled: boolean;
|
||||
testHarnessToken?: string;
|
||||
validateResponses: boolean;
|
||||
};
|
||||
presignedAttachmentUploadsEnabled: boolean;
|
||||
presignedDownloadsEnabled: boolean;
|
||||
|
||||
@@ -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<HonoEnv>, schema: ZodType): Promise<void> {
|
||||
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<T extends ZodType>(
|
||||
schema: T,
|
||||
options?: {
|
||||
@@ -39,49 +80,13 @@ export function ResponseType<T extends ZodType>(
|
||||
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);
|
||||
};
|
||||
}
|
||||
@@ -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<HonoEnv>();
|
||||
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<HonoEnv>();
|
||||
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<HonoEnv>();
|
||||
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<HonoEnv>();
|
||||
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'});
|
||||
});
|
||||
});
|
||||
@@ -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<Record<string, unknown>>(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)
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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<MessageSearchResponse>(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, '')
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user