From 1ce29544fe863d6fc99a08ebfe1e89c073eb36f0 Mon Sep 17 00:00:00 2001 From: Pratham Debnath <125961992+isthatpratham@users.noreply.github.com> Date: Mon, 31 Aug 2026 04:52:34 +0530 Subject: [PATCH 1/2] feat: add request IDs and a JSON logger Honor or generate X-Request-Id on every request and keep a small structured logger that never serializes passwords or file contents. Co-authored-by: Cursor --- src/__tests__/helmetCors.test.ts | 1 + src/__tests__/requestId.test.ts | 42 +++++++++++++++++++++ src/app.ts | 4 +- src/middleware/requestId.ts | 17 +++++++++ src/types/express.d.ts | 9 +++++ src/utils/logger.ts | 63 ++++++++++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/requestId.test.ts create mode 100644 src/middleware/requestId.ts create mode 100644 src/types/express.d.ts create mode 100644 src/utils/logger.ts diff --git a/src/__tests__/helmetCors.test.ts b/src/__tests__/helmetCors.test.ts index d3116f6..bdbbe32 100644 --- a/src/__tests__/helmetCors.test.ts +++ b/src/__tests__/helmetCors.test.ts @@ -39,6 +39,7 @@ describe('Helmet and CORS', () => { expect(res.headers['access-control-allow-origin']).toBe('http://localhost:5173'); expect(res.headers['access-control-expose-headers']?.toLowerCase()).toContain('content-disposition'); + expect(res.headers['access-control-expose-headers']?.toLowerCase()).toContain('x-request-id'); }); it('does not allow an unlisted origin', async () => { diff --git a/src/__tests__/requestId.test.ts b/src/__tests__/requestId.test.ts new file mode 100644 index 0000000..d189f87 --- /dev/null +++ b/src/__tests__/requestId.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it, vi } from 'vitest'; +import request from 'supertest'; +import app from '../app.js'; +import { resolveRequestId } from '../middleware/requestId.js'; +import { serializeLog } from '../utils/logger.js'; + +describe('request IDs', () => { + it('generates an X-Request-Id when the client does not send one', async () => { + const res = await request(app).get('/api/health'); + expect(res.headers['x-request-id']).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + ); + }); + + it('preserves an incoming X-Request-Id', async () => { + const res = await request(app).get('/api/health').set('X-Request-Id', 'client-trace-123'); + expect(res.headers['x-request-id']).toBe('client-trace-123'); + }); + + it('strips newlines from an incoming request id', () => { + expect(resolveRequestId('abc\n{"injected":true}')).toBe('abc{"injected":true}'); + }); +}); + +describe('structured logger', () => { + it('writes JSON and drops password fields', () => { + const spy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const line = serializeLog('info', { + event: 'upload_success', + fileId: 'file-1', + password: 'secret', + } as never); + + const payload = JSON.parse(line) as Record; + expect(payload.event).toBe('upload_success'); + expect(payload.fileId).toBe('file-1'); + expect(payload.password).toBeUndefined(); + expect(payload.level).toBe('info'); + expect(typeof payload.ts).toBe('string'); + spy.mockRestore(); + }); +}); diff --git a/src/app.ts b/src/app.ts index fffd28b..7827086 100644 --- a/src/app.ts +++ b/src/app.ts @@ -10,6 +10,7 @@ import { parseTrustProxy } from './config/trustProxy.js'; import { createApiRateLimiter } from './middleware/rateLimits.js'; import { isOriginAllowed, resolveCorsOrigin } from './config/cors.js'; import { securityHeaders } from './config/helmet.js'; +import { requestIdMiddleware } from './middleware/requestId.js'; dotenv.config(); @@ -18,6 +19,7 @@ const app: Application = express(); app.set('trust proxy', parseTrustProxy(process.env.TRUST_PROXY)); // Middlewares +app.use(requestIdMiddleware); app.use(securityHeaders()); app.use(express.json()); const corsOrigin = resolveCorsOrigin(process.env.CORS_ORIGIN); @@ -25,7 +27,7 @@ app.use(cors({ origin: (requestOrigin, callback) => { callback(null, isOriginAllowed(requestOrigin, corsOrigin)); }, - exposedHeaders: ['Content-Disposition'], + exposedHeaders: ['Content-Disposition', 'X-Request-Id'], })); // Routes diff --git a/src/middleware/requestId.ts b/src/middleware/requestId.ts new file mode 100644 index 0000000..8179b0a --- /dev/null +++ b/src/middleware/requestId.ts @@ -0,0 +1,17 @@ +import { randomUUID } from 'crypto'; +import type { NextFunction, Request, Response } from 'express'; + +export const REQUEST_ID_HEADER = 'x-request-id'; +const MAX_REQUEST_ID_LENGTH = 128; + +export const resolveRequestId = (incoming: string | undefined): string => { + const sanitized = (incoming ?? '').trim().replace(/[\r\n\t]/g, '').slice(0, MAX_REQUEST_ID_LENGTH); + return sanitized.length > 0 ? sanitized : randomUUID(); +}; + +export const requestIdMiddleware = (req: Request, res: Response, next: NextFunction): void => { + const requestId = resolveRequestId(req.header('X-Request-Id')); + req.requestId = requestId; + res.setHeader('X-Request-Id', requestId); + next(); +}; diff --git a/src/types/express.d.ts b/src/types/express.d.ts new file mode 100644 index 0000000..741c588 --- /dev/null +++ b/src/types/express.d.ts @@ -0,0 +1,9 @@ +declare global { + namespace Express { + interface Request { + requestId?: string; + } + } +} + +export {}; diff --git a/src/utils/logger.ts b/src/utils/logger.ts new file mode 100644 index 0000000..e4c6566 --- /dev/null +++ b/src/utils/logger.ts @@ -0,0 +1,63 @@ +const FORBIDDEN_KEYS = new Set([ + 'password', + 'password_hash', + 'authorization', + 'cookie', + 'body', + 'file', + 'contents', +]); + +export type LogLevel = 'info' | 'warn' | 'error'; + +export type LogFields = { + event: string; + requestId?: string; + method?: string; + path?: string; + status?: number; + fileId?: string; + size?: number; + expiredDeleted?: number; + orphanedDeleted?: number; + signal?: string; + message?: string; +}; + +const write = (level: LogLevel, line: string): void => { + if (level === 'error') { + console.error(line); + return; + } + if (level === 'warn') { + console.warn(line); + return; + } + console.log(line); +}; + +export const serializeLog = (level: LogLevel, fields: LogFields): string => { + const payload: Record = { + ts: new Date().toISOString(), + level, + }; + + for (const [key, value] of Object.entries(fields)) { + if (value === undefined || FORBIDDEN_KEYS.has(key.toLowerCase())) { + continue; + } + payload[key] = typeof value === 'string' && value.length > 500 ? value.slice(0, 500) : value; + } + + return JSON.stringify(payload); +}; + +export const log = (level: LogLevel, fields: LogFields): void => { + write(level, serializeLog(level, fields)); +}; + +export const requestContext = (req: { requestId?: string; method?: string; path?: string }) => ({ + requestId: req.requestId, + method: req.method, + path: req.path, +}); From 74633497f2094142be9857db7bbc8abc6c5c0e37 Mon Sep 17 00:00:00 2001 From: Pratham Debnath <125961992+isthatpratham@users.noreply.github.com> Date: Mon, 31 Aug 2026 04:54:00 +0530 Subject: [PATCH 2/2] feat: emit JSON logs for upload, download, cleanup, and lifecycle Record success, failure, password rejects, 429s, 5xx, cleanup, and startup/shutdown without logging secrets or file contents. Co-authored-by: Cursor --- src/config/db.ts | 13 +++++------ src/controllers/fileController.ts | 37 ++++++++++++++++++++----------- src/middleware/errorHandler.ts | 4 ++++ src/middleware/rateLimits.ts | 2 ++ src/server.ts | 14 +++++------- src/services/cleanupService.ts | 27 +++++++++++----------- 6 files changed, 54 insertions(+), 43 deletions(-) diff --git a/src/config/db.ts b/src/config/db.ts index 38ecb64..17b2468 100644 --- a/src/config/db.ts +++ b/src/config/db.ts @@ -1,18 +1,15 @@ import dotenv from 'dotenv'; -import { initializeSqlite, getSqlitePath } from '../../backend/database/sqlite-setup.js'; +import { initializeSqlite } from '../../backend/database/sqlite-setup.js'; +import { log } from '../utils/logger.js'; dotenv.config(); const connectDB = async (): Promise => { try { initializeSqlite(); - console.log(`SQLite Connected: ${getSqlitePath()}`); - } catch (error) { - if (error instanceof Error) { - console.error(`SQLite connection error: ${error.message}`); - } else { - console.error('An unknown error occurred during SQLite connection'); - } + log('info', { event: 'startup', message: 'sqlite_ready' }); + } catch { + log('error', { event: 'startup', message: 'sqlite_failed' }); process.exit(1); } }; diff --git a/src/controllers/fileController.ts b/src/controllers/fileController.ts index b394b2f..772dff6 100644 --- a/src/controllers/fileController.ts +++ b/src/controllers/fileController.ts @@ -7,6 +7,7 @@ import { v4 as uuidv4 } from 'uuid'; import { formatContentDisposition } from '../utils/disposition.js'; import { validateFileMagicBytes } from '../utils/fileValidation.js'; import { parseExpiryMinutes, parseMaxDownloads } from '../utils/uploadConstraints.js'; +import { log, requestContext } from '../utils/logger.js'; const removeUploadedFile = (filePath?: string): void => { if (filePath && fs.existsSync(filePath)) { @@ -32,6 +33,7 @@ const uuidV4Pattern = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[ export const uploadFile = async (req: Request, res: Response): Promise => { try { if (!req.file) { + log('warn', { event: 'upload_fail', ...requestContext(req), status: 400, message: 'No file uploaded' }); res.status(400).json({ success: false, message: 'No file uploaded' }); return; } @@ -39,6 +41,7 @@ export const uploadFile = async (req: Request, res: Response): Promise => const expiry = parseExpiryMinutes(req.body.expiryMinutes || req.body.expiry); if (!expiry.ok) { removeUploadedFile(req.file.path); + log('warn', { event: 'upload_fail', ...requestContext(req), status: 400, message: expiry.message }); res.status(400).json({ success: false, message: expiry.message }); return; } @@ -46,6 +49,7 @@ export const uploadFile = async (req: Request, res: Response): Promise => const downloads = parseMaxDownloads(req.body.maxDownloads); if (!downloads.ok) { removeUploadedFile(req.file.path); + log('warn', { event: 'upload_fail', ...requestContext(req), status: 400, message: downloads.message }); res.status(400).json({ success: false, message: downloads.message }); return; } @@ -53,6 +57,7 @@ export const uploadFile = async (req: Request, res: Response): Promise => const validation = validateFileMagicBytes(req.file.path, req.file.mimetype); if (!validation.valid) { removeUploadedFile(req.file.path); + log('warn', { event: 'upload_fail', ...requestContext(req), status: 400, message: validation.message || 'Invalid file content' }); res.status(400).json({ success: false, message: validation.message || 'Invalid file content' }); return; } @@ -89,6 +94,7 @@ export const uploadFile = async (req: Request, res: Response): Promise => passwordHash ); + log('info', { event: 'upload_success', ...requestContext(req), status: 201, fileId, size: req.file.size }); res.status(201).json({ success: true, fileId, @@ -96,6 +102,7 @@ export const uploadFile = async (req: Request, res: Response): Promise => }); } catch { removeUploadedFile(req.file?.path); + log('error', { event: 'upload_fail', ...requestContext(req), status: 500 }); res.status(500).json({ success: false, message: 'An unknown error occurred' }); } }; @@ -104,6 +111,7 @@ export const downloadFile = async (req: Request, res: Response): Promise = const fileIdParam = req.params.id; const fileId = Array.isArray(fileIdParam) ? fileIdParam[0] : fileIdParam; if (!uuidV4Pattern.test(fileId)) { + log('warn', { event: 'download_fail', ...requestContext(req), status: 404 }); res.status(404).json({ success: false, message: 'File not found' }); return; } @@ -113,6 +121,7 @@ export const downloadFile = async (req: Request, res: Response): Promise = const file = getFile.get(fileId) as SqliteFileRow | undefined; if (!file) { + log('warn', { event: 'download_fail', ...requestContext(req), status: 410, fileId }); res.status(410).json({ success: false, message: 'File has expired or is no longer available' }); return; } @@ -123,17 +132,19 @@ export const downloadFile = async (req: Request, res: Response): Promise = fs.unlinkSync(file.file_path); } db.prepare('DELETE FROM files WHERE id = ?').run(fileId); - } catch (err) { - console.error('Error deleting file:', err); + } catch { + log('error', { event: 'download_fail', ...requestContext(req), fileId, message: 'delete_failed' }); } }; if (Date.now() > new Date(file.expires_at).getTime()) { + log('warn', { event: 'download_fail', ...requestContext(req), status: 410, fileId, message: 'expired' }); res.status(410).json({ success: false, message: 'File has expired and is no longer available' }); return; } if (file.download_count >= file.max_downloads) { + log('warn', { event: 'download_fail', ...requestContext(req), status: 410, fileId, message: 'limit_reached' }); res.status(410).json({ success: false, message: 'Download limit reached' }); return; } @@ -141,11 +152,13 @@ export const downloadFile = async (req: Request, res: Response): Promise = if (file.password_hash) { const providedPassword = typeof req.body?.password === 'string' ? req.body.password : ''; if (!providedPassword) { + log('warn', { event: 'password_fail', ...requestContext(req), status: 403, fileId, message: 'required' }); res.status(403).json({ success: false, message: 'Password required' }); return; } const isMatch = await bcrypt.compare(providedPassword, file.password_hash); if (!isMatch) { + log('warn', { event: 'password_fail', ...requestContext(req), status: 403, fileId, message: 'incorrect' }); res.status(403).json({ success: false, message: 'Incorrect password' }); return; } @@ -162,6 +175,7 @@ export const downloadFile = async (req: Request, res: Response): Promise = `).run(fileId, new Date().toISOString()); if (reservation.changes === 0) { + log('warn', { event: 'download_fail', ...requestContext(req), status: 410, fileId, message: 'reservation_lost' }); res.status(410).json({ success: false, message: 'File has expired or is no longer available' }); return; } @@ -172,7 +186,7 @@ export const downloadFile = async (req: Request, res: Response): Promise = res.sendFile(absolutePath, async (err) => { if (err) { - console.error('Error sending file:', err); + log('error', { event: 'download_fail', ...requestContext(req), status: 500, fileId }); if (!res.headersSent) { res.status(500).json({ success: false, message: 'Error downloading file' }); } @@ -182,14 +196,14 @@ export const downloadFile = async (req: Request, res: Response): Promise = return; } + log('info', { event: 'download_success', ...requestContext(req), status: 200, fileId }); if (reservedCount >= file.max_downloads) { await deleteFile(); } }); - } catch (error) { - if (error instanceof Error) { - res.status(500).json({ success: false, message: error.message }); - } else { + } catch { + log('error', { event: 'download_fail', ...requestContext(req), status: 500 }); + if (!res.headersSent) { res.status(500).json({ success: false, message: 'An unknown error occurred' }); } } @@ -231,11 +245,8 @@ export const getFileInfo = async (req: Request, res: Response): Promise => createdAt: file.created_at, }, }); - } catch (error) { - if (error instanceof Error) { - res.status(500).json({ success: false, message: error.message }); - } else { - res.status(500).json({ success: false, message: 'An unknown error occurred' }); - } + } catch { + log('error', { event: 'server_error', ...requestContext(req), status: 500, path: req.path }); + res.status(500).json({ success: false, message: 'An unknown error occurred' }); } }; diff --git a/src/middleware/errorHandler.ts b/src/middleware/errorHandler.ts index 76f19de..f68523d 100644 --- a/src/middleware/errorHandler.ts +++ b/src/middleware/errorHandler.ts @@ -1,6 +1,7 @@ import { NextFunction, Request, Response } from 'express'; import fs from 'fs'; import multer from 'multer'; +import { log, requestContext } from '../utils/logger.js'; const removeUploadedFile = (req: Request): void => { const uploaded = req.file; @@ -21,14 +22,17 @@ export const errorHandler = (err: unknown, req: Request, res: Response, next: Ne const message = err.code === 'LIMIT_FILE_SIZE' ? 'File exceeds the 10MB limit' : err.message; + log('warn', { event: 'upload_fail', ...requestContext(req), status: 400, message }); res.status(400).json({ success: false, message }); return; } if (err instanceof Error && err.message === 'Invalid file type') { + log('warn', { event: 'upload_fail', ...requestContext(req), status: 400, message: 'Invalid file type' }); res.status(400).json({ success: false, message: 'Invalid file type' }); return; } + log('error', { event: 'server_error', ...requestContext(req), status: 500 }); res.status(500).json({ success: false, message: 'An unknown error occurred' }); }; diff --git a/src/middleware/rateLimits.ts b/src/middleware/rateLimits.ts index fa65266..b7f07a7 100644 --- a/src/middleware/rateLimits.ts +++ b/src/middleware/rateLimits.ts @@ -1,4 +1,5 @@ import rateLimit, { type Options } from 'express-rate-limit'; +import { log, requestContext } from '../utils/logger.js'; const parsePositiveInt = (raw: string | undefined, fallback: number): number => { const value = Number(raw); @@ -6,6 +7,7 @@ const parsePositiveInt = (raw: string | undefined, fallback: number): number => }; const jsonExceededHandler: Options['handler'] = (req, res, _next, options) => { + log('warn', { event: 'rate_limited', ...requestContext(req), status: options.statusCode }); res.status(options.statusCode).json({ success: false, message: 'Too many requests' }); }; diff --git a/src/server.ts b/src/server.ts index be4a788..2e8cde5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3,6 +3,7 @@ import app from './app.js'; import connectDB from './config/db.js'; import { startCleanupJob, stopCleanupJob } from './services/cleanupService.js'; import { closeSqlite } from '../backend/database/sqlite-setup.js'; +import { log } from './utils/logger.js'; dotenv.config(); @@ -15,22 +16,19 @@ connectDB(); startCleanupJob(); const server = app.listen(PORT, () => { - console.log(`Server running in ${process.env.NODE_ENV || 'development'} mode on port ${PORT}`); + log('info', { event: 'startup', message: `listening on ${PORT}` }); }); export const gracefulShutdown = (signal: string, callback?: () => void) => { - console.log(`Received ${signal}. Initiating graceful shutdown...`); + log('info', { event: 'shutdown', signal }); stopCleanupJob(); server.close(() => { - console.log('HTTP server closed.'); - try { closeSqlite(); - console.log('SQLite connection closed.'); - } catch (err) { - console.error('Error closing SQLite database:', err); + } catch { + log('error', { event: 'shutdown', signal, message: 'sqlite_close_failed' }); } if (callback) { @@ -41,7 +39,7 @@ export const gracefulShutdown = (signal: string, callback?: () => void) => { }); setTimeout(() => { - console.error('Forcefully shutting down server due to timeout.'); + log('error', { event: 'shutdown', signal, message: 'timeout' }); if (!callback) { process.exit(1); } diff --git a/src/services/cleanupService.ts b/src/services/cleanupService.ts index a3dfd6f..208ba6e 100644 --- a/src/services/cleanupService.ts +++ b/src/services/cleanupService.ts @@ -2,6 +2,7 @@ import cron, { ScheduledTask } from 'node-cron'; import fs from 'fs'; import path from 'path'; import { getSqliteDb, getUploadDir } from '../../backend/database/sqlite-setup.js'; +import { log } from '../utils/logger.js'; type FileRow = { id: string; @@ -37,7 +38,7 @@ export const reconcileStorageDirectory = (): number => { // Path Safety / Containment Check: ensure file is directly inside uploadDir if (!filePath.startsWith(uploadDir)) { - console.warn(`Path safety check failed during reconciliation for: ${filePath}`); + log('warn', { event: 'cleanup', message: 'path_safety_rejected' }); continue; } @@ -56,16 +57,15 @@ export const reconcileStorageDirectory = (): number => { if (!activePathsSet.has(filePath)) { fs.unlinkSync(filePath); unlinkedCount++; - console.log(`Reconciled orphaned storage file deleted: ${filename}`); } - } catch (fileErr) { - console.error(`Error processing file during storage reconciliation ${filename}:`, fileErr); + } catch { + log('error', { event: 'cleanup', message: 'reconcile_file_failed' }); } } return unlinkedCount; - } catch (error) { - console.error('Error during storage directory reconciliation:', error); + } catch { + log('error', { event: 'cleanup', message: 'reconcile_failed' }); return 0; } }; @@ -89,17 +89,18 @@ export const performCleanupRound = (): { expiredDeleted: number; orphanedDeleted } db.prepare('DELETE FROM files WHERE id = ?').run(file.id); expiredDeleted++; - } catch (fileError) { - console.error(`Failed to clean up record ${file.id}:`, fileError); + } catch { + log('error', { event: 'cleanup', fileId: file.id, message: 'record_cleanup_failed' }); } } // 2. Reconcile orphaned files on disk const orphanedDeleted = reconcileStorageDirectory(); + log('info', { event: 'cleanup', expiredDeleted, orphanedDeleted }); return { expiredDeleted, orphanedDeleted }; - } catch (error) { - console.error('Error during cleanup round:', error); + } catch { + log('error', { event: 'cleanup', message: 'round_failed' }); return { expiredDeleted: 0, orphanedDeleted: 0 }; } }; @@ -111,9 +112,7 @@ export const startCleanupJob = (): ScheduledTask => { // Run every 5 minutes scheduledTask = cron.schedule('*/5 * * * *', () => { - console.log('Running scheduled cleanup job...'); - const result = performCleanupRound(); - console.log(`Cleanup job summary: ${result.expiredDeleted} database records cleaned, ${result.orphanedDeleted} orphaned files removed.`); + performCleanupRound(); }); return scheduledTask; @@ -123,6 +122,6 @@ export const stopCleanupJob = (): void => { if (scheduledTask) { scheduledTask.stop(); scheduledTask = null; - console.log('Cleanup background cron job stopped.'); + log('info', { event: 'cleanup', message: 'stopped' }); } };