feat: structured body-parse error log and client-IP extraction helper

Issue #288 — src/middlewares/body-parse-error.middleware.ts
- New error-handler middleware that intercepts body-parsing failures
  (SyntaxError with body, entity.too.large) exclusively on mutation
  methods (POST, PUT, PATCH, DELETE)
- Emits logger.error with: type, method, path, requestId, clientIp,
  errorType — never logs the raw body or request headers
- Returns 400 / 413 JSON to the client unchanged; non-parse errors and
  GET/HEAD/OPTIONS pass straight through to next()
- Mounted in app.ts immediately after express.json() so parse errors are
  caught before any route or the global error handler sees them
- 14 tests covering: all mutation verbs, GET passthrough, generic errors,
  entity.too.large, no-body-in-log assertion, client response shape

Issue #289 — src/utils/client-ip.utils.ts
- getClientIp(req, trusted?) reads the first IP from X-Forwarded-For when
  the socket address belongs to a trusted proxy (loopback, RFC-1918, IPv6
  ULA); falls back to req.socket.remoteAddress otherwise
- Accepts an optional predicate override for testing and custom deployments
- Integrated into request-logger.middleware.ts (clientIp field in every
  request log entry) and body-parse-error.middleware.ts (clientIp in
  parse-failure logs)
- 11 tests: trusted loopback/private ranges, array header, custom predicate,
  untrusted socket passthrough, missing socket edge cases

Closes #288
Closes #289

Author: ijeoma270

src/app.ts

@@ -15,6 +15,7 @@ import { apiVersionMiddleware } from './middlewares/api-version.middleware';
 import { schemaVersionMiddleware } from './middlewares/schema-version.middleware';
 import { requestLoggerMiddleware } from './middlewares/request-logger.middleware';
 import { requestContextMiddleware } from './middlewares/request-context.middleware';
+import { bodyParseErrorMiddleware } from './middlewares/body-parse-error.middleware';
 import { envConfig } from './config';
 
 const app: Express = express();
@@ -29,6 +30,7 @@ app.use(requestIdMiddleware);
 app.use(corsMiddleware());
 app.use(helmet());
 app.use(express.json({ limit: '10mb' }));
+app.use(bodyParseErrorMiddleware);
 
 if (!envConfig.ENABLE_REQUEST_LOGGING) {
    app.use(morgan('combined'));

src/middlewares/body-parse-error.middleware.test.ts

@@ -0,0 +1,169 @@
+import { Request, Response, NextFunction } from 'express';
+import { bodyParseErrorMiddleware } from './body-parse-error.middleware';
+import { logger } from '../utils/logger.utils';
+
+jest.mock('../utils/logger.utils', () => ({
+  logger: { error: jest.fn() },
+}));
+
+jest.mock('../utils/client-ip.utils', () => ({
+  getClientIp: jest.fn(() => '10.0.0.1'),
+}));
+
+function makeReq(method: string, url = '/api/v1/resource'): Request {
+  return {
+    method,
+    originalUrl: url,
+    url,
+    requestId: 'req-test-id',
+    socket: { remoteAddress: '127.0.0.1' },
+    headers: {},
+  } as unknown as Request;
+}
+
+function makeRes(): Partial<Response> {
+  const res: Partial<Response> = {};
+  res.status = jest.fn().mockReturnValue(res);
+  res.json = jest.fn().mockReturnValue(res);
+  return res;
+}
+
+function makeSyntaxError(): SyntaxError & { body: string } {
+  const err = new SyntaxError('Unexpected token } in JSON') as SyntaxError & { body: string };
+  err.body = 'raw body';
+  return err;
+}
+
+function makeEntityTooLargeError() {
+  return Object.assign(new Error('request entity too large'), {
+    type: 'entity.too.large',
+    status: 413,
+    limit: 10 * 1024 * 1024,
+  });
+}
+
+describe('bodyParseErrorMiddleware', () => {
+  let next: jest.Mock;
+
+  beforeEach(() => {
+    next = jest.fn();
+    jest.clearAllMocks();
+  });
+
+  describe('malformed JSON on mutation methods', () => {
+    const MUTATION_METHODS = ['POST', 'PUT', 'PATCH', 'DELETE'];
+
+    it.each(MUTATION_METHODS)(
+      'logs error and returns 400 for %s with invalid JSON',
+      (method) => {
+        const err = makeSyntaxError();
+        const req = makeReq(method);
+        const res = makeRes();
+
+        bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+        expect(logger.error).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'body_parse_failure',
+            method,
+            path: '/api/v1/resource',
+            requestId: 'req-test-id',
+            errorType: 'invalid_json',
+          })
+        );
+        expect(res.status).toHaveBeenCalledWith(400);
+        expect(res.json).toHaveBeenCalledWith(
+          expect.objectContaining({ success: false, message: 'Invalid JSON in request body' })
+        );
+        expect(next).not.toHaveBeenCalled();
+      }
+    );
+
+    it('does not include raw body in the log', () => {
+      const err = makeSyntaxError();
+      const req = makeReq('POST');
+      const res = makeRes();
+
+      bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+      const logCall = (logger.error as jest.Mock).mock.calls[0][0];
+      expect(logCall).not.toHaveProperty('body');
+      expect(logCall).not.toHaveProperty('rawBody');
+    });
+
+    it('logs and returns 413 for entity.too.large on mutation', () => {
+      const err = makeEntityTooLargeError();
+      const req = makeReq('POST');
+      const res = makeRes();
+
+      bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+      expect(logger.error).toHaveBeenCalledWith(
+        expect.objectContaining({ errorType: 'entity.too.large' })
+      );
+      expect(res.status).toHaveBeenCalledWith(413);
+    });
+  });
+
+  describe('non-mutation methods', () => {
+    it('calls next() for GET with a SyntaxError (not a mutation)', () => {
+      const err = makeSyntaxError();
+      const req = makeReq('GET');
+      const res = makeRes();
+
+      bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+      expect(next).toHaveBeenCalledWith(err);
+      expect(logger.error).not.toHaveBeenCalled();
+    });
+
+    it('calls next() for HEAD with a SyntaxError', () => {
+      const err = makeSyntaxError();
+      const req = makeReq('HEAD');
+      const res = makeRes();
+
+      bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+      expect(next).toHaveBeenCalledWith(err);
+    });
+  });
+
+  describe('non-parse errors', () => {
+    it('calls next() for a generic Error on a mutation method', () => {
+      const err = new Error('Something else entirely');
+      const req = makeReq('POST');
+      const res = makeRes();
+
+      bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+      expect(next).toHaveBeenCalledWith(err);
+      expect(logger.error).not.toHaveBeenCalled();
+    });
+
+    it('calls next() for an ApiError on a mutation method', () => {
+      const err = Object.assign(new Error('Not found'), { statusCode: 404 });
+      const req = makeReq('PUT');
+      const res = makeRes();
+
+      bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+      expect(next).toHaveBeenCalledWith(err);
+    });
+  });
+
+  describe('client response behaviour unchanged', () => {
+    it('returns 400 JSON with success:false and does not call next', () => {
+      const err = makeSyntaxError();
+      const req = makeReq('POST');
+      const res = makeRes();
+
+      bodyParseErrorMiddleware(err, req, res as Response, next as NextFunction);
+
+      expect(res.status).toHaveBeenCalledWith(400);
+      expect((res.json as jest.Mock).mock.calls[0][0]).toMatchObject({
+        success: false,
+      });
+      expect(next).not.toHaveBeenCalled();
+    });
+  });
+});

src/middlewares/body-parse-error.middleware.ts

@@ -0,0 +1,68 @@
+import { Request, Response, NextFunction } from 'express';
+import { logger } from '../utils/logger.utils';
+import { getClientIp } from '../utils/client-ip.utils';
+import { ErrorCode } from '../constants/error.constants';
+
+const MUTATION_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']);
+
+/**
+ * Intercepts body-parsing errors on mutation endpoints (POST, PUT, PATCH,
+ * DELETE) and emits a structured error-level log entry before returning the
+ * client a 400 response.
+ *
+ * What IS logged:
+ *   - endpoint path
+ *   - HTTP method
+ *   - request ID (for correlation)
+ *   - client IP (extracted via the trusted-proxy-aware helper)
+ *   - error type / code
+ *
+ * What is NOT logged:
+ *   - raw request body (never read or forwarded)
+ *   - request headers beyond what Express already exposes on req
+ */
+export const bodyParseErrorMiddleware = (
+  err: any,
+  req: Request,
+  res: Response,
+  next: NextFunction
+): void => {
+  const isSyntaxError =
+    err instanceof SyntaxError && 'body' in err;
+  const isEntityTooLarge =
+    err.type === 'entity.too.large' ||
+    err.status === 413 ||
+    err.statusCode === 413;
+
+  const isParseFailure = isSyntaxError || isEntityTooLarge;
+
+  if (!isParseFailure || !MUTATION_METHODS.has(req.method)) {
+    return next(err);
+  }
+
+  const clientIp = getClientIp(req);
+
+  logger.error({
+    type: 'body_parse_failure',
+    method: req.method,
+    path: req.originalUrl || req.url,
+    requestId: req.requestId,
+    clientIp,
+    errorType: isEntityTooLarge ? 'entity.too.large' : 'invalid_json',
+  });
+
+  if (isEntityTooLarge) {
+    res.status(413).json({
+      success: false,
+      code: ErrorCode.BAD_REQUEST,
+      message: 'Request payload too large',
+    });
+    return;
+  }
+
+  res.status(400).json({
+    success: false,
+    code: ErrorCode.BAD_REQUEST,
+    message: 'Invalid JSON in request body',
+  });
+};

src/middlewares/request-logger.middleware.ts

@@ -3,6 +3,7 @@ import { Request, Response, NextFunction } from 'express';
 import { envConfig } from '../config';
 import { logger } from '../utils/logger.utils';
 import { computeRequestContextHash } from '../utils/request-context-hash.utils';
+import { getClientIp } from '../utils/client-ip.utils';
 
 /**
  * Lightweight request logging middleware.
@@ -39,6 +40,7 @@ export const requestLoggerMiddleware = (
          status: res.statusCode,
          duration: `${durationMs}ms`,
          requestId: req.requestId,
+         clientIp: getClientIp(req),
          contextHash,
       });
    });

src/utils/client-ip.utils.test.ts

@@ -0,0 +1,96 @@
+import { getClientIp } from './client-ip.utils';
+import { Request } from 'express';
+
+function makeReq(
+  socketIp: string,
+  forwardedFor?: string
+): Request {
+  return {
+    socket: { remoteAddress: socketIp },
+    headers: forwardedFor ? { 'x-forwarded-for': forwardedFor } : {},
+  } as unknown as Request;
+}
+
+describe('getClientIp', () => {
+  describe('when socket is a trusted proxy', () => {
+    it('returns the first X-Forwarded-For IP', () => {
+      const req = makeReq('127.0.0.1', '203.0.113.5, 10.0.0.1');
+      expect(getClientIp(req)).toBe('203.0.113.5');
+    });
+
+    it('handles a single IP in X-Forwarded-For', () => {
+      const req = makeReq('10.0.0.2', '198.51.100.7');
+      expect(getClientIp(req)).toBe('198.51.100.7');
+    });
+
+    it('trims whitespace from the extracted IP', () => {
+      const req = makeReq('192.168.1.1', '  1.2.3.4  , 10.0.0.5');
+      expect(getClientIp(req)).toBe('1.2.3.4');
+    });
+
+    it('falls back to socket address when X-Forwarded-For is absent', () => {
+      const req = makeReq('10.10.10.1');
+      expect(getClientIp(req)).toBe('10.10.10.1');
+    });
+
+    it('falls back to socket address when X-Forwarded-For is empty string', () => {
+      const req = makeReq('172.16.0.1', '');
+      expect(getClientIp(req)).toBe('172.16.0.1');
+    });
+
+    it('handles array-valued X-Forwarded-For header', () => {
+      const req = {
+        socket: { remoteAddress: '127.0.0.1' },
+        headers: { 'x-forwarded-for': ['9.9.9.9, 10.0.0.1', '8.8.8.8'] },
+      } as unknown as Request;
+      expect(getClientIp(req)).toBe('9.9.9.9');
+    });
+
+    it('accepts private 172.16.x.x as a trusted proxy', () => {
+      const req = makeReq('172.16.5.10', '55.55.55.55');
+      expect(getClientIp(req)).toBe('55.55.55.55');
+    });
+
+    it('accepts IPv6 loopback ::1 as trusted proxy', () => {
+      const req = makeReq('::1', '203.0.113.99');
+      expect(getClientIp(req)).toBe('203.0.113.99');
+    });
+  });
+
+  describe('when socket is NOT a trusted proxy', () => {
+    it('ignores X-Forwarded-For and returns the socket address', () => {
+      const req = makeReq('203.0.113.1', '1.2.3.4');
+      expect(getClientIp(req)).toBe('203.0.113.1');
+    });
+
+    it('returns socket address even when no forwarded header is present', () => {
+      const req = makeReq('8.8.8.8');
+      expect(getClientIp(req)).toBe('8.8.8.8');
+    });
+  });
+
+  describe('custom trusted predicate', () => {
+    it('uses the override predicate instead of the default CIDR check', () => {
+      const req = makeReq('8.8.8.8', '203.0.113.42');
+      // Treat all IPs as trusted via override
+      expect(getClientIp(req, () => true)).toBe('203.0.113.42');
+    });
+
+    it('rejects all proxies when override always returns false', () => {
+      const req = makeReq('127.0.0.1', '203.0.113.42');
+      expect(getClientIp(req, () => false)).toBe('127.0.0.1');
+    });
+  });
+
+  describe('edge cases', () => {
+    it('returns undefined when socket address is missing', () => {
+      const req = { socket: {}, headers: {} } as unknown as Request;
+      expect(getClientIp(req)).toBeUndefined();
+    });
+
+    it('returns undefined when socket itself is undefined', () => {
+      const req = { headers: {} } as unknown as Request;
+      expect(getClientIp(req)).toBeUndefined();
+    });
+  });
+});