Pin the request body ceiling instead of inheriting it

The Express shell set a 100kb limit and stopped applying it the moment the
Nest instance took over parsing: bootstrap passed bodyParser:false and
nothing replaced it. The number happened to stay the same, because it is also
the framework default - but it lived nowhere anybody would look for it, and
one NestFactory default away from silently becoming something else.

It goes through Nest's own parser rather than a second express.json(),
because that parser carries the verify hook that keeps req.rawBody, and the
plugin webhook routes need the exact payload bytes to check a provider's
HMAC. Adding a parser in front of it would have taken rawBody away without
failing anything.

MISC-009 and MISC-010 pin it from both sides.

The bodyParser flag on applyGlobalMiddleware goes with it: one caller, which
passed false.

Not moved: cookieParser stays on the raw Express layer rather than becoming a
configure(consumer) middleware. The platform routes - the OAuth consent flow
among them - are registered on that instance before Nest routing exists, so a
Nest-slot cookie parser would simply not run for them. helmet, CORS,
compression, the forced-HTTPS 301 and the res.on('finish') logging stay for
the same reason: they have to see requests Nest never routes.
This commit is contained in:
Maurice
2026-08-10 11:35:19 +02:00
committed by Maurice
parent be8081c3b4
commit e5c99903ea
4 changed files with 39 additions and 11 deletions
+12 -2
View File
@@ -1,5 +1,5 @@
import { NestFactory } from '@nestjs/core';
import { ExpressAdapter } from '@nestjs/platform-express';
import { ExpressAdapter, type NestExpressApplication } from '@nestjs/platform-express';
import type { INestApplication } from '@nestjs/common';
import type { ConfigType } from '@nestjs/config';
import { AppModule } from './nest/app.module';
@@ -54,10 +54,20 @@ export async function buildApp(): Promise<INestApplication> {
// the one bridge that lets the pre-init Express layer consume the validated
// config instead of reading process.env itself.
const http = app.get<ConfigType<typeof httpConfig>>(httpConfig.KEY);
applyGlobalMiddleware(instance, { bodyParser: false, http });
applyGlobalMiddleware(instance, { http });
applyPlatformUploads(instance);
applyPlatformTransport(instance);
applyPlatformStatic(instance);
// Pin the request-body ceiling explicitly. The Express shell used to set
// '100kb' and stopped doing it when the Nest instance took over parsing, which
// left the limit implicit — the same number, but nowhere anybody would find
// it, and one NestFactory default away from silently becoming something else.
// Configured through Nest rather than a second express.json(): its parser
// carries the verify hook that keeps req.rawBody, which the plugin webhook
// routes need to check a provider's HMAC over the exact payload.
const express = app as unknown as NestExpressApplication;
express.useBodyParser('json', { limit: '100kb' });
express.useBodyParser('urlencoded', { limit: '100kb', extended: true });
if (apiDocsEnabled()) setupApiDocs(app);
await app.init();
// The /mcp handler is mounted pre-init (step 3) and has no DI access — hand
+5 -8
View File
@@ -15,18 +15,19 @@ import { logDebug, logWarn, logError } from '../nest/audit/audit-log.logger';
* behaviourally transparent — and is the prerequisite for retiring Express,
* since the Nest instance must carry the whole shell on its own.
*
* `bodyParser` is opt-out: the Nest instance does its own body parsing, so it
* passes `false` to avoid parsing the request twice.
* Body parsing is NOT here: the Nest instance does it, with the limit pinned in
* bootstrap.ts. The flag that used to switch a second parser on had exactly one
* caller, which passed false.
*/
export function applyGlobalMiddleware(
app: express.Application,
opts: { bodyParser?: boolean; http?: AppEnv['http'] } = {},
opts: { http?: AppEnv['http'] } = {},
): void {
// The whole pipeline is configured at APPLY time (the per-request closures
// capture these values), so a snapshot is the correct semantic. bootstrap
// threads in the DI-loaded httpConfig; direct callers fall back to an
// apply-time readEnv() — same values, same freeze point.
const { bodyParser = true, http = readEnv().http } = opts;
const { http = readEnv().http } = opts;
const { nodeEnv, isProduction } = readEnv().app;
// Trust first proxy (nginx/Docker) for correct req.ip
@@ -169,10 +170,6 @@ export function applyGlobalMiddleware(
});
}
if (bodyParser) {
app.use(express.json({ limit: '100kb' }));
app.use(express.urlencoded({ extended: true }));
}
app.use(cookieParser());
// Request logging with sensitive field redaction
+21
View File
@@ -145,3 +145,24 @@ describe('Force HTTPS redirect', () => {
});
});
describe('Request body ceiling', () => {
// The Express shell set '100kb' and stopped when the Nest instance took over
// parsing, which left the limit implicit. This pins it, so it cannot drift
// back to a framework default without somebody noticing.
it('MISC-009 — a body over 100kb is refused with 413', async () => {
const res = await request(app)
.post('/api/auth/login')
.set('Content-Type', 'application/json')
.send({ email: 'a@b.c', password: 'x'.repeat(200 * 1024) });
expect(res.status).toBe(413);
});
it('MISC-010 — a body under it still reaches the handler', async () => {
const res = await request(app)
.post('/api/auth/login')
.set('Content-Type', 'application/json')
.send({ email: 'a@b.c', password: 'x'.repeat(1024) });
expect(res.status).not.toBe(413);
});
});
@@ -5,7 +5,7 @@ import { applyGlobalMiddleware } from '../../../src/middleware/globalMiddleware'
async function connectSrcSources(): Promise<string[]> {
const app = express();
applyGlobalMiddleware(app, { bodyParser: false });
applyGlobalMiddleware(app);
app.get('/probe', (_req, res) => res.json({ ok: true }));
const res = await request(app).get('/probe');