Skip to content

Commit 24a8574

Browse files
feat(logging): Winston structured logging, AuditLog module, and Sentry integration (#3318)
* feat(logging): add Winston structured logging, AuditLog module, and Sentry integration - Winston JSON logging with configurable levels + X-Request-ID middleware - AuditLog module with model, service, routes, policy, controller, and tests - Sentry SDK integration with request context enrichment - Enriched health endpoint with version, uptime, and dependency checks - Config: log.json, log.level, sentry.dsn/environment/enabled, audit.ttlDays/enabled * fix(logging): address review feedback — validation, layer fixes, JSDoc - Validate X-Request-ID header to prevent log injection - Mount requestId middleware before pre-parser routes - Add orgId morgan token for org-scoped request correlation - Fix Sentry dynamic import to destructure default export - Add Zod validation in AuditService.log before persisting - Validate userId/orgId as ObjectId in Zod schemas - Return 400 on invalid audit query params instead of silent fallback - Guard deleteMany against empty/missing filter - Move health endpoint DB logic to HomeService (layer separation) - Use response envelope in health endpoint - Move health test to home module tests - Add JSDoc @returns and @param annotations * refactor(audit): replace manual controller instrumentation with auto-capture middleware Move audit logging from individual controller calls to a global Express middleware that hooks into res.on('finish'), matching the existing analytics middleware pattern. This removes AuditService imports from auth, billing, and organizations controllers while preserving the same audit coverage via automatic route-based action derivation. * fix(audit): address CodeRabbit review — reject blank action, use Winston logger, add 503 test - AuditQuery.action now uses .min(1) to reject empty/whitespace-only strings - AuditService error paths use Winston logger instead of console.error - Added integration test for degraded health (503) response path - Mock logger in audit service unit tests to avoid winston config dependency * fix(health): protect detailed response behind admin auth Public /api/health returns only { status } for K8s probes. Admin users (via JWT cookie) get full details: db, uptime, version, memory. Uses passport optional auth pattern (custom callback, never rejects). * fix(audit): improve JSDoc @returns type for list method Document the paginated result shape {data, total, page, perPage} for better developer experience. * fix(audit): move validation to repository, add JSDoc, restore config in tests - Move Zod schema validation from service to repository layer (proper dependency direction) - Add @returns to optionalAuth and route factory JSDoc in home.route.js - Restore config.organizations.enabled in afterAll to prevent cross-test leak
1 parent 15ce95b commit 24a8574

34 files changed

Lines changed: 2381 additions & 19 deletions

config/defaults/development.config.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,11 @@ const config = {
5050
// logging with Morgan - https://github.com/expressjs/morgan
5151
// Can specify one of 'combined', 'common', 'dev', 'short', 'tiny', 'custom'
5252
format: 'custom',
53-
pattern: ':id :email :method :url :status :response-time ms - :res[content-length]', // only for custom format
53+
pattern: ':requestId :id :email :method :url :status :response-time ms - :res[content-length]', // only for custom format
54+
// Structured JSON output for Winston console transport (enable in production)
55+
json: false,
56+
// Winston log level: error, warn, info, http, verbose, debug, silly
57+
level: 'info',
5458
fileLogger: {
5559
directoryPath: process.cwd(),
5660
fileName: 'app.log',
@@ -90,6 +94,11 @@ const config = {
9094
trust: {
9195
proxy: false,
9296
},
97+
sentry: {
98+
dsn: process.env.DEVKIT_NODE_sentry_dsn || '',
99+
environment: process.env.DEVKIT_NODE_sentry_environment || 'development',
100+
enabled: false,
101+
},
93102
domain: '',
94103
cookie: {
95104
secure: false, // false in dev (HTTP localhost)

config/defaults/production.config.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,14 @@ const config = {
3939
log: {
4040
format: 'custom',
4141
pattern:
42-
':id :email :remote-addr - :remote-user [:date[clf]] ":method :url HTTP/:http-version" :status :res[content-length] ":referrer" ":user-agent"', // only for custom format
42+
':requestId :id :email :remote-addr - :remote-user [:date[clf]] ":method :url HTTP/:http-version" :status :res[content-length] ":referrer" ":user-agent"', // only for custom format
43+
json: true,
44+
level: 'info',
45+
},
46+
sentry: {
47+
dsn: process.env.DEVKIT_NODE_sentry_dsn || '',
48+
environment: 'production',
49+
enabled: !!process.env.DEVKIT_NODE_sentry_dsn,
4350
},
4451
};
4552

config/defaults/test.config.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ const config = {
99
uri: 'mongodb://127.0.0.1:27017/NodeTest',
1010
debug: false,
1111
},
12+
audit: {
13+
enabled: true,
14+
ttlDays: 1,
15+
},
16+
sentry: {
17+
dsn: '',
18+
enabled: false,
19+
},
1220
organizations: {
1321
enabled: false,
1422
domainMatching: false,

jest.config.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ export default {
4343
'!<rootDir>/modules/auth/auth.init.js',
4444
// Exclude analytics init glue — just calls AnalyticsService.init()
4545
'!<rootDir>/modules/analytics/analytics.init.js',
46+
// Exclude audit init glue — just logs activation status
47+
'!<rootDir>/modules/audit/audit.init.js',
4648
// Exclude dead code — never imported anywhere in the codebase
4749
'!<rootDir>/modules/users/services/users.data.service.js',
4850
// Exclude server bootstrap — startup orchestration, tested indirectly via integration tests
@@ -177,7 +179,7 @@ export default {
177179
// testLocationInResults: false,
178180

179181
// The glob patterns Jest uses to detect test files
180-
testMatch: ['<rootDir>/modules/*/tests/**/*.js'],
182+
testMatch: ['<rootDir>/modules/*/tests/**/*.js', '<rootDir>/lib/**/tests/**/*.js'],
181183

182184
// An array of regexp pattern strings that are matched against all test paths, matched tests are skipped
183185
testPathIgnorePatterns: ['/node_modules/'],

lib/app.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import express from './services/express.js';
1010
import mongooseService from './services/mongoose.js';
1111
import migrations from './services/migrations.js';
1212
import AnalyticsService from '../modules/analytics/services/analytics.service.js';
13+
import SentryService from './services/sentry.js';
1314

1415
// Establish an SQL server connection, instantiating all models and schemas
1516
// const startSequelize = () =>
@@ -70,6 +71,7 @@ const bootstrap = async () => {
7071
// }
7172

7273
try {
74+
await SentryService.init();
7375
db = await startMongoose();
7476
await migrations.run();
7577
app = await startExpress();
@@ -149,6 +151,7 @@ const shutdown = async (server) => {
149151
try {
150152
const value = await server;
151153
await AnalyticsService.shutdown();
154+
await SentryService.shutdown();
152155
await mongooseService.disconnect();
153156
value.http.close((err) => {
154157
console.info(chalk.yellow('Server closed'));

lib/middlewares/policy.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ const deriveSubjectType = (routePath) => {
197197
if (routePath === '/api/billing/subscription') return 'BillingSubscription';
198198
if (routePath === '/api/billing/usage') return 'BillingUsage';
199199
if (routePath === '/api/billing/plans') return 'BillingPlans';
200+
if (routePath.startsWith('/api/audit')) return 'AuditLog';
200201
if (routePath.startsWith('/api/tasks')) return 'Task';
201202
if (routePath.startsWith('/api/uploads')) return 'Upload';
202203
if (routePath.startsWith('/api/home')) return 'Home';

lib/middlewares/requestId.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* Module dependencies
3+
*/
4+
import { randomUUID } from 'crypto';
5+
6+
/**
7+
* Validate that a request ID is safe for use in logs and headers.
8+
* Accepts alphanumeric characters, dashes, and underscores up to 128 chars.
9+
* @param {string} id - Candidate request ID
10+
* @returns {boolean} true if the ID is valid
11+
*/
12+
const isValidRequestId = (id) => typeof id === 'string' && id.length <= 128 && /^[\w-]+$/.test(id);
13+
14+
/**
15+
* Express middleware that assigns a unique request ID to every incoming request.
16+
* If the client sends a valid `X-Request-ID` header, that value is reused;
17+
* otherwise a new UUID v4 is generated. The ID is exposed as `req.id` and
18+
* echoed back via the `X-Request-ID` response header for end-to-end tracing.
19+
*
20+
* @param {Object} req - Express request object
21+
* @param {Object} res - Express response object
22+
* @param {Function} next - Express next middleware function
23+
*/
24+
const requestId = (req, res, next) => {
25+
const clientId = req.headers['x-request-id'];
26+
const id = (clientId && isValidRequestId(clientId)) ? clientId : randomUUID();
27+
req.id = id;
28+
res.setHeader('X-Request-ID', id);
29+
next();
30+
};
31+
32+
export default requestId;
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Module dependencies.
3+
*/
4+
import { jest, describe, test, expect, beforeEach } from '@jest/globals';
5+
import requestId from '../requestId.js';
6+
7+
describe('requestId middleware unit tests:', () => {
8+
let req;
9+
let res;
10+
let next;
11+
12+
beforeEach(() => {
13+
req = { headers: {} };
14+
res = {
15+
_headers: {},
16+
setHeader(key, value) {
17+
this._headers[key] = value;
18+
},
19+
};
20+
next = jest.fn();
21+
});
22+
23+
test('should generate a UUID when no X-Request-ID header is present', () => {
24+
requestId(req, res, next);
25+
26+
expect(req.id).toBeDefined();
27+
expect(typeof req.id).toBe('string');
28+
// UUID v4 format
29+
expect(req.id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/);
30+
expect(res._headers['X-Request-ID']).toBe(req.id);
31+
expect(next).toHaveBeenCalledTimes(1);
32+
});
33+
34+
test('should reuse X-Request-ID from incoming request header', () => {
35+
req.headers['x-request-id'] = 'custom-id-123';
36+
37+
requestId(req, res, next);
38+
39+
expect(req.id).toBe('custom-id-123');
40+
expect(res._headers['X-Request-ID']).toBe('custom-id-123');
41+
expect(next).toHaveBeenCalledTimes(1);
42+
});
43+
44+
test('should call next exactly once', () => {
45+
requestId(req, res, next);
46+
expect(next).toHaveBeenCalledTimes(1);
47+
});
48+
});

lib/services/express.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import swaggerUi from 'swagger-ui-express';
1919

2020
import config from '../../config/index.js';
2121
import logger from './logger.js';
22+
import requestId from '../middlewares/requestId.js';
23+
import sentry from './sentry.js';
2224

2325
/**
2426
* Initialize Swagger
@@ -103,6 +105,8 @@ const initMiddleware = (app) => {
103105
if (_.has(config, 'log.format') && process.env.NODE_ENV !== 'test') {
104106
morgan.token('id', (req) => _.get(req, 'user.id') || 'Unknown id');
105107
morgan.token('email', (req) => _.get(req, 'user.email') || 'Unknown email');
108+
morgan.token('requestId', (req) => req.id || '-');
109+
morgan.token('orgId', (req) => _.get(req, 'organization.id') || _.get(req, 'organization._id', '-'));
106110
app.use(morgan(logger.getLogFormat(), logger.getMorganOptions()));
107111
}
108112
// Request body parsing middleware should be above methodOverride
@@ -208,6 +212,8 @@ const init = async () => {
208212
initSwagger(app);
209213
// Initialize local variables
210214
initLocalVariables(app);
215+
// Assign a unique request ID before any route registration
216+
app.use(requestId);
211217
// Initialize pre-parser routes (before body parsing and CSRF)
212218
await initPreParserRoutes(app);
213219
// Initialize Express middleware
@@ -224,6 +230,8 @@ const init = async () => {
224230
await initModulesServerPolicies(app);
225231
// Initialize modules server routes
226232
await initModulesServerRoutes(app);
233+
// Mount Sentry error handler (must be after routes, before generic error handler)
234+
sentry.setupExpressErrorHandler(app);
227235
// Initialize error routes
228236
initErrorRoutes(app);
229237
return app;

lib/services/logger.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,28 @@ import config from '../../config/index.js';
77
// list of valid formats for the logging
88
const validFormats = ['combined', 'common', 'dev', 'short', 'tiny', 'custom'];
99

10+
// Determine log level from config (falls back to 'info')
11+
const logLevel = _.get(config, 'log.level', 'info');
12+
13+
// Determine whether to use structured JSON output
14+
const useJson = _.get(config, 'log.json', false);
15+
16+
// Build console transport format
17+
const consoleFormat = useJson
18+
? winston.format.combine(winston.format.timestamp(), winston.format.json())
19+
: winston.format.combine(winston.format.colorize(), winston.format.simple());
20+
1021
// Instantiating the default winston application logger with the Console
1122
// transport
1223

1324
/* eslint new-cap: 0 */
1425
const logger = new winston.createLogger({
26+
level: logLevel,
1527
transports: [
1628
new winston.transports.Console({
17-
level: 'info',
18-
colorize: true,
19-
showLevel: true,
29+
level: logLevel,
30+
format: consoleFormat,
2031
handleExceptions: true,
21-
humanReadableUnhandledException: true,
2232
}),
2333
],
2434
exitOnError: false,

0 commit comments

Comments
 (0)