fix(api): answer 403 when the client ip header is unparsable (#2483)

fluxer_api/src/api/app/MiddlewarePipeline.ts

@@ -91,11 +91,7 @@ export function configureMiddleware(routes: HonoApp, options: MiddlewarePipeline
 	}
 	routes.use(TorExitMiddleware);
 	routes.use(AuditLogMiddleware);
-	routes.use(
+	routes.use(RequireClientIpMiddleware());
-		RequireClientIpMiddleware({
-			requiredHeaders: [resolvedHeader],
-		}),
-	);
 	routes.use(ServiceMiddleware);
 	routes.use(UserMiddleware);
 	routes.use(ContentFilterMiddleware);

fluxer_api/src/api/middleware/RequireClientIpMiddleware.ts

@@ -2,16 +2,15 @@
 
 import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes';
 import {ForbiddenError} from '@fluxer/errors/src/domains/core/ForbiddenError';
-import {resolveClientIpHeaderName} from '@fluxer/ip_utils/src/ClientIp';
 import {createMiddleware} from 'hono/factory';
 import {Config} from '../Config';
 import {Logger} from '../Logger';
 import type {HonoEnv} from '../types/HonoEnv';
+import {getRequestClientIp} from '../utils/RequestClientIp';
 import {stripApiPrefix} from '../utils/RequestPathUtils';
 
 interface RequireClientIpOptions {
 	exemptPaths?: Array<string>;
-	requiredHeaders?: Array<string>;
 }
 
 const defaultExemptPaths: Array<string> = [
@@ -22,10 +21,7 @@ const defaultExemptPaths: Array<string> = [
 	'/connections/bluesky/jwks.json',
 ];
 
-export function RequireClientIpMiddleware({
+export function RequireClientIpMiddleware({exemptPaths = defaultExemptPaths}: RequireClientIpOptions = {}) {
-	exemptPaths = defaultExemptPaths,
-	requiredHeaders = [resolveClientIpHeaderName(Config.proxy.client_ip_header)],
-}: RequireClientIpOptions = {}) {
 	return createMiddleware<HonoEnv>(async (ctx, next) => {
 		if (Config.dev.testModeEnabled) {
 			await next();
@@ -36,12 +32,8 @@ export function RequireClientIpMiddleware({
 			await next();
 			return;
 		}
-		const hasRequiredHeader = requiredHeaders.some((header) => {
+		if (getRequestClientIp(ctx) === null) {
-			const value = ctx.req.header(header);
+			Logger.warn({path}, 'Rejected request without a resolvable client IP');
-			return value != null && value.trim() !== '';
-		});
-		if (!hasRequiredHeader) {
-			Logger.warn({path}, 'Rejected request without required proxy headers');
 			throw new ForbiddenError({code: APIErrorCodes.FORBIDDEN});
 		}
 		await next();

fluxer_api/src/api/middleware/tests/RequireClientIpMiddleware.test.ts

@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {AppErrorHandler} from '@fluxer/errors/src/domains/core/ErrorHandlers';
+import {Hono} from 'hono';
+import {afterEach, beforeEach, describe, expect, it} from 'vitest';
+import {Config} from '../../Config';
+import type {HonoEnv} from '../../types/HonoEnv';
+import type {ClientIpResolution} from '../../utils/RequestClientIp';
+import {RequireClientIpMiddleware} from '../RequireClientIpMiddleware';
+
+interface Harness {
+	request: (headers: Record<string, string>) => Promise<Response>;
+	resolutions: Array<ClientIpResolution | undefined>;
+}
+
+function createHarness(path = 'http://localhost/v1/messages'): Harness {
+	const resolutions: Array<ClientIpResolution | undefined> = [];
+	const app = new Hono<HonoEnv>();
+	app.use(RequireClientIpMiddleware());
+	app.get('/v1/messages', (ctx) => {
+		resolutions.push(ctx.get('clientIpResolution'));
+		return ctx.text('ok');
+	});
+	app.get('/_health', (ctx) => ctx.text('OK'));
+	app.onError(AppErrorHandler);
+	return {
+		request: async (headers) => app.request(path, {headers}),
+		resolutions,
+	};
+}
+
+describe('RequireClientIpMiddleware', () => {
+	let previousTestModeEnabled: boolean;
+	let previousTrustClientIpHeader: boolean;
+
+	beforeEach(() => {
+		previousTestModeEnabled = Config.dev.testModeEnabled;
+		previousTrustClientIpHeader = Config.proxy.trust_client_ip_header;
+		Config.dev.testModeEnabled = false;
+		Config.proxy.trust_client_ip_header = true;
+	});
+
+	afterEach(() => {
+		Config.dev.testModeEnabled = previousTestModeEnabled;
+		Config.proxy.trust_client_ip_header = previousTrustClientIpHeader;
+	});
+
+	it('rejects an unparsable client ip header with 403 FORBIDDEN', async () => {
+		const harness = createHarness();
+		const response = await harness.request({'x-forwarded-for': 'not-an-ip'});
+		expect(response.status).toBe(403);
+		expect(await response.json()).toMatchObject({code: 'FORBIDDEN'});
+	});
+
+	it('rejects a request when the header is not trusted with 403', async () => {
+		Config.proxy.trust_client_ip_header = false;
+		const harness = createHarness();
+		const response = await harness.request({'x-forwarded-for': '203.0.113.10'});
+		expect(response.status).toBe(403);
+		expect(await response.json()).toMatchObject({code: 'FORBIDDEN'});
+	});
+
+	it('rejects a request with no client ip header with 403 FORBIDDEN', async () => {
+		const harness = createHarness();
+		const response = await harness.request({});
+		expect(response.status).toBe(403);
+		expect(await response.json()).toMatchObject({code: 'FORBIDDEN'});
+	});
+
+	it('passes a parsable header and caches the resolution', async () => {
+		const harness = createHarness();
+		const response = await harness.request({'x-forwarded-for': '203.0.113.10, 10.0.0.1'});
+		expect(response.status).toBe(200);
+		expect(harness.resolutions[0]?.ip).toBe('203.0.113.10');
+	});
+
+	it('leaves exempt paths alone', async () => {
+		const harness = createHarness('http://localhost/_health');
+		const response = await harness.request({});
+		expect(response.status).toBe(200);
+	});
+
+	it('passes every request through in test mode', async () => {
+		Config.dev.testModeEnabled = true;
+		const harness = createHarness();
+		const response = await harness.request({'x-forwarded-for': 'not-an-ip'});
+		expect(response.status).toBe(200);
+	});
+});

fluxer_api/src/api/utils/tests/RequestClientIp.test.ts

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 
+import {AppErrorHandler} from '@fluxer/errors/src/domains/core/ErrorHandlers';
 import {MissingClientIpError} from '@fluxer/ip_utils/src/ClientIp';
 import {Hono} from 'hono';
 import {describe, expect, it} from 'vitest';
@@ -125,4 +126,15 @@ describe('RequestClientIp', () => {
 		await app.request('http://localhost/v1/messages', {headers: {'x-forwarded-for': '203.0.113.10'}});
 		expect(seen).toEqual(['203.0.113.10', null, '203.0.113.10']);
 	});
+	it('a missing client ip surfaces as 403 FORBIDDEN through AppErrorHandler', async () => {
+		const app = new Hono<HonoEnv>();
+		app.get('/v1/messages', (ctx) => {
+			requireRequestClientIp(ctx);
+			return ctx.text('ok');
+		});
+		app.onError(AppErrorHandler);
+		const response = await app.request('http://localhost/v1/messages');
+		expect(response.status).toBe(403);
+		expect(await response.json()).toMatchObject({code: 'FORBIDDEN'});
+	});
 });

packages/ip_utils/src/ClientIp.ts

@@ -21,6 +21,9 @@ interface HeadersLike {
 }
 
 export class MissingClientIpError extends Error {
+	readonly code = 'FORBIDDEN';
+	readonly status = 403;
+
 	constructor() {
 		super('Client IP header is required');
 		this.name = 'MissingClientIpError';