diff --git a/README.md b/README.md index 4d76cb7..386fdb8 100644 --- a/README.md +++ b/README.md @@ -173,11 +173,16 @@ The backend includes abuse-oriented security instrumentation middleware. - `security.failed_login_attempt` - `security.rate_limit_triggered` - `security.suspicious_pattern` +- Rate-limit logs include the limiter profile, request method, request path, threshold, and window without logging raw API keys or user identifiers. ### Thresholds (env-configurable) | Env var | Default | Meaning | |---|---|---| +| `ANALYTICS_RATE_LIMIT_WINDOW_MS` | `900000` | Analytics read rate-limit window | +| `ANALYTICS_RATE_LIMIT_MAX` | `120` | Max analytics requests per client key or IP in the analytics window | +| `MUTATION_RATE_LIMIT_WINDOW_MS` | `900000` | Mutation endpoint rate-limit window for `POST`, `PUT`, `PATCH`, `DELETE` | +| `MUTATION_RATE_LIMIT_MAX` | `40` | Max mutation requests per client key or IP in the mutation window | | `SECURITY_RATE_LIMIT_WINDOW_MS` | `60000` | Rate-limit lookback window | | `SECURITY_RATE_LIMIT_MAX_REQUESTS` | `120` | Max requests per IP in rate-limit window | | `SECURITY_SUSPICIOUS_WINDOW_MS` | `300000` | Lookback window for suspicious pattern checks | diff --git a/data/disciplr.db b/data/disciplr.db index db7a745..99e27b9 100644 Binary files a/data/disciplr.db and b/data/disciplr.db differ diff --git a/data/disciplr.db-shm b/data/disciplr.db-shm deleted file mode 100644 index 21cdb81..0000000 Binary files a/data/disciplr.db-shm and /dev/null differ diff --git a/data/disciplr.db-wal b/data/disciplr.db-wal deleted file mode 100644 index 6728ee2..0000000 Binary files a/data/disciplr.db-wal and /dev/null differ diff --git a/package-lock.json b/package-lock.json index 2bea8b6..2512357 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1759,7 +1759,7 @@ }, "node_modules/@prisma/config": { "version": "6.19.2", - "dev": true, + "devOptional": true, "license": "Apache-2.0", "dependencies": { "c12": "3.1.0", @@ -1770,12 +1770,12 @@ }, "node_modules/@prisma/debug": { "version": "6.19.2", - "dev": true, + "devOptional": true, "license": "Apache-2.0" }, "node_modules/@prisma/engines": { "version": "6.19.2", - "dev": true, + "devOptional": true, "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { @@ -1787,12 +1787,12 @@ }, "node_modules/@prisma/engines-version": { "version": "7.1.1-3.c2990dca591cba766e3b7ef5d9e8a84796e47ab7", - "dev": true, + "devOptional": true, "license": "Apache-2.0" }, "node_modules/@prisma/fetch-engine": { "version": "6.19.2", - "dev": true, + "devOptional": true, "license": "Apache-2.0", "dependencies": { "@prisma/debug": "6.19.2", @@ -1802,7 +1802,7 @@ }, "node_modules/@prisma/get-platform": { "version": "6.19.2", - "dev": true, + "devOptional": true, "license": "Apache-2.0", "dependencies": { "@prisma/debug": "6.19.2" @@ -2185,7 +2185,7 @@ }, "node_modules/@standard-schema/spec": { "version": "1.1.0", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/@stellar/js-xdr": { @@ -3683,7 +3683,7 @@ }, "node_modules/c12": { "version": "3.1.0", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "chokidar": "^4.0.3", @@ -3710,7 +3710,7 @@ }, "node_modules/c12/node_modules/dotenv": { "version": "16.6.1", - "dev": true, + "devOptional": true, "license": "BSD-2-Clause", "engines": { "node": ">=12" @@ -3832,7 +3832,7 @@ }, "node_modules/chokidar": { "version": "4.0.3", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "readdirp": "^4.0.1" @@ -3866,7 +3866,7 @@ }, "node_modules/citty": { "version": "0.1.6", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "consola": "^3.2.3" @@ -4029,12 +4029,12 @@ }, "node_modules/confbox": { "version": "0.2.4", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/consola": { "version": "3.4.2", - "dev": true, + "devOptional": true, "license": "MIT", "engines": { "node": "^14.18.0 || >=16.10.0" @@ -4178,7 +4178,7 @@ }, "node_modules/deepmerge-ts": { "version": "7.1.5", - "dev": true, + "devOptional": true, "license": "BSD-3-Clause", "engines": { "node": ">=16.0.0" @@ -4201,7 +4201,7 @@ }, "node_modules/defu": { "version": "6.1.4", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/delayed-stream": { @@ -4220,7 +4220,7 @@ }, "node_modules/destr": { "version": "2.0.5", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/destroy": { @@ -4300,7 +4300,7 @@ }, "node_modules/effect": { "version": "3.18.4", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "@standard-schema/spec": "^1.0.0", @@ -4334,7 +4334,7 @@ }, "node_modules/empathic": { "version": "2.0.0", - "dev": true, + "devOptional": true, "license": "MIT", "engines": { "node": ">=14" @@ -4798,12 +4798,12 @@ }, "node_modules/exsolve": { "version": "1.0.8", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/fast-check": { "version": "3.23.2", - "dev": true, + "devOptional": true, "funding": [ { "type": "individual", @@ -5158,7 +5158,7 @@ }, "node_modules/giget": { "version": "2.0.0", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "citty": "^0.1.6", @@ -6347,7 +6347,7 @@ }, "node_modules/jiti": { "version": "2.6.1", - "dev": true, + "devOptional": true, "license": "MIT", "bin": { "jiti": "lib/jiti-cli.mjs" @@ -6901,7 +6901,7 @@ }, "node_modules/node-fetch-native": { "version": "1.6.7", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/node-int64": { @@ -6941,7 +6941,7 @@ }, "node_modules/nypm": { "version": "0.6.5", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "citty": "^0.2.0", @@ -6957,7 +6957,7 @@ }, "node_modules/nypm/node_modules/citty": { "version": "0.2.1", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/object-assign": { @@ -6988,7 +6988,7 @@ }, "node_modules/ohash": { "version": "2.0.11", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/on-finished": { @@ -7183,12 +7183,12 @@ }, "node_modules/pathe": { "version": "2.0.3", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/perfect-debounce": { "version": "1.0.0", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/pg": { @@ -7368,7 +7368,7 @@ }, "node_modules/pkg-types": { "version": "2.3.0", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "confbox": "^0.2.2", @@ -7503,7 +7503,7 @@ }, "node_modules/prisma": { "version": "6.19.2", - "dev": true, + "devOptional": true, "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { @@ -7558,7 +7558,7 @@ }, "node_modules/pure-rand": { "version": "6.1.0", - "dev": true, + "devOptional": true, "funding": [ { "type": "individual", @@ -7633,7 +7633,7 @@ }, "node_modules/rc9": { "version": "2.1.2", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "defu": "^6.1.4", @@ -7661,7 +7661,7 @@ }, "node_modules/readdirp": { "version": "4.1.2", - "dev": true, + "devOptional": true, "license": "MIT", "engines": { "node": ">= 14.18.0" @@ -8491,7 +8491,7 @@ }, "node_modules/tinyexec": { "version": "1.0.2", - "dev": true, + "devOptional": true, "license": "MIT", "engines": { "node": ">=18" @@ -8769,7 +8769,7 @@ }, "node_modules/typescript": { "version": "5.9.3", - "dev": true, + "devOptional": true, "license": "Apache-2.0", "bin": { "tsc": "bin/tsc", diff --git a/package.json b/package.json index 49e105c..1adf3b8 100644 --- a/package.json +++ b/package.json @@ -8,8 +8,8 @@ "build": "tsc", "start": "node dist/index.js", "lint": "eslint src --ext .ts", - "test": "node --experimental-vm-modules node_modules/jest/bin/jest.js", - "test:watch": "node --experimental-vm-modules node_modules/jest/bin/jest.js --watch", + "test": "node --experimental-vm-modules node_modules/jest/bin/jest.js --config jest.config.ts", + "test:watch": "node --experimental-vm-modules node_modules/jest/bin/jest.js --config jest.config.ts --watch", "test:api-keys": "tsx --test src/routes/apiKeys.test.ts", "test:vaults": "tsx --test src/routes/vaults.test.ts", "migrate:make": "knex migrate:make --knexfile knexfile.cjs --migrations-directory db/migrations --extension cjs", diff --git a/src/index.ts b/src/index.ts index 054a17d..73de5b8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,7 +5,12 @@ import { createJobsRouter } from './routes/jobs.js' import { BackgroundJobSystem } from './jobs/system.js' import { authRouter } from './routes/auth.js' import { analyticsRouter } from './routes/analytics.js' -import { healthRateLimiter, vaultsRateLimiter } from './middleware/rateLimiter.js' +import { + analyticsRateLimiter, + healthRateLimiter, + mutationRateLimiter, + vaultsRateLimiter, +} from './middleware/rateLimiter.js' import { createExportRouter } from './routes/exports.js' import { transactionsRouter } from './routes/transactions.js' import { privacyRouter } from './routes/privacy.js' @@ -37,13 +42,14 @@ app.use(securityMetricsMiddleware) app.use(securityRateLimitMiddleware) app.use('/api/health', healthRateLimiter, createHealthRouter(jobSystem)) +app.use('/api', mutationRateLimiter) app.use('/api/jobs', createJobsRouter(jobSystem)) app.use('/api/vaults', vaultsRateLimiter, vaultsRouter) app.use('/api/vaults/:vaultId/milestones', milestonesRouter) app.use('/api/auth', authRouter) app.use('/api/exports', createExportRouter([])) app.use('/api/transactions', transactionsRouter) -app.use('/api/analytics', analyticsRouter) +app.use('/api/analytics', analyticsRateLimiter, analyticsRouter) app.use('/api/privacy', privacyRouter) app.use('/api/organizations', orgVaultsRouter) app.use('/api/organizations', orgAnalyticsRouter) diff --git a/src/middleware/rateLimiter.ts b/src/middleware/rateLimiter.ts index f2201a7..af9e52c 100644 --- a/src/middleware/rateLimiter.ts +++ b/src/middleware/rateLimiter.ts @@ -1,7 +1,8 @@ -import rateLimit from 'express-rate-limit' -import type { Request, Response } from 'express' +import rateLimit, { ipKeyGenerator } from 'express-rate-limit' +import type { Request, RequestHandler, Response } from 'express' export interface RateLimitConfig { + profile?: RateLimitProfile windowMs: number max: number message?: string @@ -12,18 +13,65 @@ export interface RateLimitConfig { handler?: (req: Request, res: Response) => void } -const logRateLimitBreached = (req: Request): void => { - const timestamp = new Date().toISOString() - const clientIp = req.ip ?? req.socket.remoteAddress ?? 'unknown' - const method = req.method - const path = req.path - const userAgent = req.headers['user-agent'] ?? 'unknown' - const apiKey = req.headers['x-api-key'] ?? 'none' +type RateLimitProfile = 'default' | 'auth' | 'health' | 'vaults' | 'strict' | 'analytics' | 'mutation' - console.warn(`[RATE_LIMIT_BREACH] ${timestamp} | IP: ${clientIp} | API_KEY: ${apiKey} | ${method} ${path} | User-Agent: ${userAgent}`) +type RateLimitMetrics = Record + +const rateLimitMetrics: RateLimitMetrics = { + default: 0, + auth: 0, + health: 0, + vaults: 0, + strict: 0, + analytics: 0, + mutation: 0, +} + +const parsePositiveIntegerEnv = (name: string, fallback: number): number => { + const rawValue = process.env[name] + + if (!rawValue) { + return fallback + } + + const parsedValue = Number.parseInt(rawValue, 10) + if (!Number.isFinite(parsedValue) || parsedValue <= 0) { + return fallback + } + + return parsedValue +} + +const incrementRateLimitMetric = (profile: RateLimitProfile): void => { + rateLimitMetrics[profile] += 1 +} + +const logRateLimitBreached = ( + req: Request, + profile: RateLimitProfile, + windowMs: number, + max: number, +): void => { + incrementRateLimitMetric(profile) + + console.warn( + JSON.stringify({ + level: 'warn', + event: 'security.rate_limit_triggered', + service: 'disciplr-backend', + profile, + method: req.method, + path: req.originalUrl || req.path, + identifierSource: typeof req.headers['x-api-key'] === 'string' ? 'api-key' : 'ip', + windowMs, + threshold: max, + timestamp: new Date().toISOString(), + }), + ) } const createRateLimiter = (config: Partial = {}) => { + const profile = config.profile ?? 'default' const windowMs = config.windowMs ?? 15 * 60 * 1000 const max = config.max ?? 100 @@ -35,10 +83,14 @@ const createRateLimiter = (config: Partial = {}) => { skipSuccessfulRequests: config.skipSuccessfulRequests ?? false, keyGenerator: config.keyGenerator ?? ((req) => { const apiKey = req.headers['x-api-key'] as string | undefined - return apiKey ?? req.ip ?? req.socket.remoteAddress ?? 'unknown' + if (apiKey) { + return apiKey + } + + return ipKeyGenerator(req.ip ?? req.socket.remoteAddress ?? 'unknown') }), handler: config.handler ?? ((req, res) => { - logRateLimitBreached(req) + logRateLimitBreached(req, profile, windowMs, max) res.status(429).json({ error: config.message ?? 'Too many requests, please try again later.', retryAfter: Math.ceil(windowMs / 1000), @@ -47,34 +99,74 @@ const createRateLimiter = (config: Partial = {}) => { }) } +const createMethodScopedRateLimiter = ( + methods: readonly string[], + limiter: RequestHandler, +): RequestHandler => { + const allowedMethods = new Set(methods.map((method) => method.toUpperCase())) + + return (req, res, next) => { + if (!allowedMethods.has(req.method.toUpperCase())) { + next() + return + } + + limiter(req, res, next) + } +} + +export const getRateLimitMetricsSnapshot = (): RateLimitMetrics => ({ ...rateLimitMetrics }) + export const defaultRateLimiter = createRateLimiter({ + profile: 'default', windowMs: 15 * 60 * 1000, max: 100, message: 'Rate limit exceeded. Please try again later.', }) export const authRateLimiter = createRateLimiter({ + profile: 'auth', windowMs: 15 * 60 * 1000, max: 20, message: 'Too many authentication attempts. Please try again later.', }) export const healthRateLimiter = createRateLimiter({ + profile: 'health', windowMs: 60 * 1000, max: 30, message: 'Health check rate limit exceeded.', }) export const vaultsRateLimiter = createRateLimiter({ + profile: 'vaults', windowMs: 15 * 60 * 1000, max: 50, message: 'Too many vault requests. Please try again later.', }) export const strictRateLimiter = createRateLimiter({ + profile: 'strict', windowMs: 60 * 60 * 1000, max: 10, message: 'Rate limit exceeded. This endpoint has strict rate limits.', }) -export { createRateLimiter } +export const analyticsRateLimiter = createRateLimiter({ + profile: 'analytics', + windowMs: parsePositiveIntegerEnv('ANALYTICS_RATE_LIMIT_WINDOW_MS', 15 * 60 * 1000), + max: parsePositiveIntegerEnv('ANALYTICS_RATE_LIMIT_MAX', 120), + message: 'Rate limit exceeded. Too many analytics requests. Please try again later.', +}) + +export const mutationRateLimiter = createMethodScopedRateLimiter( + ['POST', 'PUT', 'PATCH', 'DELETE'], + createRateLimiter({ + profile: 'mutation', + windowMs: parsePositiveIntegerEnv('MUTATION_RATE_LIMIT_WINDOW_MS', 15 * 60 * 1000), + max: parsePositiveIntegerEnv('MUTATION_RATE_LIMIT_MAX', 40), + message: 'Rate limit exceeded. Too many mutation requests. Please try again later.', + }), +) + +export { createMethodScopedRateLimiter, createRateLimiter } diff --git a/src/routes/orgAnalytics.ts b/src/routes/orgAnalytics.ts index e4d248d..0d8d76c 100644 --- a/src/routes/orgAnalytics.ts +++ b/src/routes/orgAnalytics.ts @@ -1,12 +1,14 @@ import { Router, Request, Response } from 'express' import { authenticate } from '../middleware/auth.js' import { requireOrgAccess } from '../middleware/orgAuth.js' +import { analyticsRateLimiter } from '../middleware/rateLimiter.js' import { vaults, Vault } from './vaults.js' export const orgAnalyticsRouter = Router() orgAnalyticsRouter.get( '/:orgId/analytics', + analyticsRateLimiter, authenticate, requireOrgAccess('owner', 'admin'), (req: Request, res: Response) => { diff --git a/tests/rateLimiter.test.ts b/tests/rateLimiter.test.ts new file mode 100644 index 0000000..c4e32eb --- /dev/null +++ b/tests/rateLimiter.test.ts @@ -0,0 +1,150 @@ +import express from 'express' +import { jest } from '@jest/globals' +import request from 'supertest' + +const RATE_LIMIT_ENV_KEYS = [ + 'ANALYTICS_RATE_LIMIT_WINDOW_MS', + 'ANALYTICS_RATE_LIMIT_MAX', + 'MUTATION_RATE_LIMIT_WINDOW_MS', + 'MUTATION_RATE_LIMIT_MAX', +] as const + +const ORIGINAL_ENV = { ...process.env } + +const restoreRateLimitEnv = (): void => { + for (const key of RATE_LIMIT_ENV_KEYS) { + if (ORIGINAL_ENV[key] === undefined) { + delete process.env[key] + continue + } + + process.env[key] = ORIGINAL_ENV[key] + } +} + +const loadRateLimiterModule = async () => { + jest.resetModules() + return import('../src/middleware/rateLimiter.js') +} + +describe('rate limiter profiles', () => { + beforeEach(() => { + restoreRateLimitEnv() + }) + + afterEach(() => { + restoreRateLimitEnv() + jest.restoreAllMocks() + }) + + it('enforces analytics limits from env with a stable 429 payload', async () => { + process.env.ANALYTICS_RATE_LIMIT_WINDOW_MS = '60000' + process.env.ANALYTICS_RATE_LIMIT_MAX = '1' + process.env.MUTATION_RATE_LIMIT_WINDOW_MS = '60000' + process.env.MUTATION_RATE_LIMIT_MAX = '3' + + const { analyticsRateLimiter } = await loadRateLimiterModule() + + const app = express() + app.get('/analytics', analyticsRateLimiter, (_req, res) => { + res.status(200).json({ ok: true }) + }) + + const firstResponse = await request(app).get('/analytics') + const secondResponse = await request(app).get('/analytics') + + expect(firstResponse.status).toBe(200) + expect(secondResponse.status).toBe(429) + expect(secondResponse.body).toEqual({ + error: 'Rate limit exceeded. Too many analytics requests. Please try again later.', + retryAfter: 60, + }) + }) + + it('applies mutation limits only to mutating methods', async () => { + process.env.MUTATION_RATE_LIMIT_WINDOW_MS = '60000' + process.env.MUTATION_RATE_LIMIT_MAX = '1' + + const { mutationRateLimiter } = await loadRateLimiterModule() + + const app = express() + app.use(express.json()) + app.get('/vaults', mutationRateLimiter, (_req, res) => { + res.status(200).json({ method: 'GET' }) + }) + app.post('/vaults', mutationRateLimiter, (_req, res) => { + res.status(201).json({ method: 'POST' }) + }) + + const getFirst = await request(app).get('/vaults') + const getSecond = await request(app).get('/vaults') + const postFirst = await request(app).post('/vaults').send({}) + const postSecond = await request(app).post('/vaults').send({}) + + expect(getFirst.status).toBe(200) + expect(getSecond.status).toBe(200) + expect(postFirst.status).toBe(201) + expect(postSecond.status).toBe(429) + expect(postSecond.body).toEqual({ + error: 'Rate limit exceeded. Too many mutation requests. Please try again later.', + retryAfter: 60, + }) + }) + + it('tracks analytics and mutation breaches independently', async () => { + process.env.ANALYTICS_RATE_LIMIT_WINDOW_MS = '60000' + process.env.ANALYTICS_RATE_LIMIT_MAX = '1' + process.env.MUTATION_RATE_LIMIT_WINDOW_MS = '60000' + process.env.MUTATION_RATE_LIMIT_MAX = '2' + + const { analyticsRateLimiter, mutationRateLimiter, getRateLimitMetricsSnapshot } = + await loadRateLimiterModule() + + const app = express() + app.use(express.json()) + app.get('/analytics', analyticsRateLimiter, (_req, res) => { + res.status(200).json({ ok: true }) + }) + app.post('/vaults', mutationRateLimiter, (_req, res) => { + res.status(201).json({ ok: true }) + }) + + await request(app).get('/analytics') + await request(app).get('/analytics') + await request(app).post('/vaults').send({}) + await request(app).post('/vaults').send({}) + await request(app).post('/vaults').send({}) + + expect(getRateLimitMetricsSnapshot()).toMatchObject({ + analytics: 1, + mutation: 1, + }) + }) + + it('falls back to safe defaults for invalid env values and avoids logging raw API keys', async () => { + process.env.ANALYTICS_RATE_LIMIT_WINDOW_MS = 'not-a-number' + process.env.ANALYTICS_RATE_LIMIT_MAX = '1' + + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined) + const { analyticsRateLimiter } = await loadRateLimiterModule() + + const app = express() + app.get('/analytics', analyticsRateLimiter, (_req, res) => { + res.status(200).json({ ok: true }) + }) + + await request(app).get('/analytics').set('x-api-key', 'secret-api-key-value') + const limitedResponse = await request(app) + .get('/analytics') + .set('x-api-key', 'secret-api-key-value') + + expect(limitedResponse.status).toBe(429) + expect(limitedResponse.body.retryAfter).toBe(900) + expect(warnSpy).toHaveBeenCalledTimes(1) + + const loggedMessage = String(warnSpy.mock.calls[0][0]) + expect(loggedMessage).toContain('security.rate_limit_triggered') + expect(loggedMessage).toContain('"identifierSource":"api-key"') + expect(loggedMessage).not.toContain('secret-api-key-value') + }) +})