From 33302d7f7377456a03c4bff6af160b281cf3d443 Mon Sep 17 00:00:00 2001 From: zaytherr Date: Sun, 18 Oct 2026 03:33:50 +0100 Subject: [PATCH 1/4] feat(server): enforce HTTPS and add HSTS headers (#468) --- .env.example | 18 +++ README.md | 7 + src/config.js | 41 ++++++ src/index.js | 12 ++ src/middleware/httpsEnforcement.js | 167 +++++++++++++++++++++ test/httpsEnforcement.test.js | 225 +++++++++++++++++++++++++++++ 6 files changed, 470 insertions(+) create mode 100644 src/middleware/httpsEnforcement.js create mode 100644 test/httpsEnforcement.test.js diff --git a/.env.example b/.env.example index 1f98f40..27d841b 100644 --- a/.env.example +++ b/.env.example @@ -142,3 +142,21 @@ INDEXER_MAX_POLL_INTERVAL_MS=300000 INDEXER_BACKOFF_FACTOR=2 # INDEXER_LAG_ALERT_THRESHOLD: ledgers behind the tip before lag is alerted. Default: 100. INDEXER_LAG_ALERT_THRESHOLD=100 + +# HTTPS enforcement and proxy trust (#468) +# TRUST_PROXY: Express trust proxy setting ('true', 'false', 'loopback', IP/subnet, or hop count). +# Default: 'loopback' in production, 'false' in development/test. +TRUST_PROXY= +# ENFORCE_HTTPS: boolean. Default: true in production, false in development/test. +ENFORCE_HTTPS=false +# HTTPS_HOST: public hostname used as target for HTTPS redirects to prevent Host header injection. +HTTPS_HOST= +# ALLOWED_HOSTS: comma-separated list of permitted hostnames for redirection. +ALLOWED_HOSTS= +# HSTS_MAX_AGE_SECONDS: Strict-Transport-Security max-age in seconds. Default: 31536000 (1 year). +HSTS_MAX_AGE_SECONDS=31536000 +# HSTS_INCLUDE_SUBDOMAINS: boolean. Default: true. +HSTS_INCLUDE_SUBDOMAINS=true +# HSTS_PRELOAD: boolean. Default: false. Strictly opt-in; removal from browser preload lists takes months. +HSTS_PRELOAD=false + diff --git a/README.md b/README.md index 29d6d73..b2710ec 100644 --- a/README.md +++ b/README.md @@ -302,6 +302,13 @@ Each price source (`stellar_dex`, `coingecko`, `coinmarketcap`) gets one circuit | `API_KEY_RATELIMIT_FREE_MAX` | Requests per window for `free`-tier keys | 100 | No | | `API_KEY_RATELIMIT_PRO_MAX` | Requests per window for `pro`-tier keys | 1000 | No | | `API_KEY_RATELIMIT_ADMIN_MAX` | Requests per window for `admin`-tier keys | 10000 | No | +| `TRUST_PROXY` | Express trust proxy setting (`true`, `false`, `loopback`, IP/subnet, or hops) | `loopback` in production, `false` otherwise | No | +| `ENFORCE_HTTPS` | Enforce HTTPS redirects (308) and reject state-changing plain HTTP | `true` in production, `false` otherwise | No | +| `HTTPS_HOST` | Fixed target public host for HTTPS redirection | empty | No | +| `ALLOWED_HOSTS` | Comma-separated allowlist of valid redirection hosts | empty | No | +| `HSTS_MAX_AGE_SECONDS` | HSTS max-age duration in seconds | 31536000 | No | +| `HSTS_INCLUDE_SUBDOMAINS` | Include subdomains in HSTS header | true | No | +| `HSTS_PRELOAD` | Opt-in to HSTS preloading | false | No | --- diff --git a/src/config.js b/src/config.js index 92b1534..63a9e91 100644 --- a/src/config.js +++ b/src/config.js @@ -67,6 +67,18 @@ const databaseDevDefault = ? 'postgres://localhost/smartdrop_test' : 'postgres://localhost/smartdrop'; +function parseTrustProxy(value, nodeEnv) { + if (value === undefined || value === '') { + return nodeEnv === 'production' ? 'loopback' : false; + } + const trimmed = String(value).trim().toLowerCase(); + if (trimmed === 'true') return true; + if (trimmed === 'false') return false; + const numVal = Number(trimmed); + if (!Number.isNaN(numVal)) return numVal; + return value; +} + const rawEnv = { ...process.env, NODE_ENV: process.env.NODE_ENV || 'development', @@ -258,6 +270,21 @@ const config = module.exports = { sentryDsn: env.SENTRY_DSN, slowRequestThresholdMs: env.SLOW_REQUEST_THRESHOLD_MS, routeTimeoutMs: env.ROUTE_TIMEOUT_MS, + trustProxy: parseTrustProxy(process.env.TRUST_PROXY, env.NODE_ENV), + https: { + enforce: process.env.ENFORCE_HTTPS !== undefined && process.env.ENFORCE_HTTPS !== '' + ? process.env.ENFORCE_HTTPS === 'true' + : env.NODE_ENV === 'production', + host: process.env.HTTPS_HOST || process.env.PUBLIC_HOSTNAME || '', + allowedHosts: (process.env.ALLOWED_HOSTS || '') + .split(',') + .map((h) => h.trim()) + .filter(Boolean), + hstsMaxAge: parseInt(process.env.HSTS_MAX_AGE_SECONDS, 10) || 31536000, + hstsIncludeSubDomains: process.env.HSTS_INCLUDE_SUBDOMAINS !== 'false', + // Preload is strictly opt-in because removal from browser preload lists takes months. + hstsPreload: process.env.HSTS_PRELOAD === 'true', + }, webhookSecretEncryptionKey: env.WEBHOOK_SECRET_ENCRYPTION_KEY, // How long browsers may cache a CORS preflight response (issue #344). // Exposed as Access-Control-Max-Age on every successful OPTIONS preflight. @@ -407,6 +434,20 @@ function reload() { sentryDsn: reloaded.SENTRY_DSN, slowRequestThresholdMs: reloaded.SLOW_REQUEST_THRESHOLD_MS, routeTimeoutMs: reloaded.ROUTE_TIMEOUT_MS, + trustProxy: parseTrustProxy(process.env.TRUST_PROXY, reloaded.NODE_ENV), + https: { + enforce: process.env.ENFORCE_HTTPS !== undefined && process.env.ENFORCE_HTTPS !== '' + ? process.env.ENFORCE_HTTPS === 'true' + : reloaded.NODE_ENV === 'production', + host: process.env.HTTPS_HOST || process.env.PUBLIC_HOSTNAME || '', + allowedHosts: (process.env.ALLOWED_HOSTS || '') + .split(',') + .map((h) => h.trim()) + .filter(Boolean), + hstsMaxAge: parseInt(process.env.HSTS_MAX_AGE_SECONDS, 10) || 31536000, + hstsIncludeSubDomains: process.env.HSTS_INCLUDE_SUBDOMAINS !== 'false', + hstsPreload: process.env.HSTS_PRELOAD === 'true', + }, webhookSecretEncryptionKey: reloaded.WEBHOOK_SECRET_ENCRYPTION_KEY, corsMaxAgeSeconds: reloaded.CORS_MAX_AGE_SECONDS, watchedAssets: reloadedWatchedAssets, diff --git a/src/index.js b/src/index.js index 23889d4..286cae5 100644 --- a/src/index.js +++ b/src/index.js @@ -47,6 +47,7 @@ const { router: metricsRouter, requestMetricsMiddleware, } = require("./routes/metrics"); +const { httpsEnforcementMiddleware } = require("./middleware/httpsEnforcement"); const healthRouter = require("./health"); const priceWebSocket = require("./ws/priceWebSocket"); @@ -83,6 +84,10 @@ const wrappedAirdropExpiryJob = makeLeaderAwareJob({ }); const app = express(); +// #468 — Configure trust proxy according to trusted proxy setting. +// Express secure-request detection (req.secure) relies on this setting. +app.set("trust proxy", config.trustProxy); + let server = { close(callback) { if (callback) callback(); @@ -93,6 +98,8 @@ app.use(headerSecurityMiddleware); app.use(requestIdMiddleware); app.use(requestLoggerMiddleware); app.use(requestMetricsMiddleware); +// #468 — Enforce HTTPS early before payload parsing or route processing. +app.use(httpsEnforcementMiddleware); app.use(compression()); app.use( helmet({ @@ -103,6 +110,9 @@ app.use( referrerPolicy: { policy: "strict-origin-when-cross-origin" }, xContentTypeOptions: true, xDownloadOptions: true, + // #468: HSTS is managed exclusively in httpsEnforcementMiddleware so it is + // sent ONLY on secure responses and matches configured max-age/subdomains/preload. + hsts: false, }) ); app.use(buildCorsMiddleware(config.corsAllowedOrigins)); @@ -449,6 +459,8 @@ function logStartupBanner() { indexer_enabled: config.indexer.enabled, instance_id: config.leaderElection.instanceId, log_level: process.env.LOG_LEVEL || config.nodeEnv, + trust_proxy: config.trustProxy, + https_enforced: config.https.enforce, }); } diff --git a/src/middleware/httpsEnforcement.js b/src/middleware/httpsEnforcement.js new file mode 100644 index 0000000..dd83de0 --- /dev/null +++ b/src/middleware/httpsEnforcement.js @@ -0,0 +1,167 @@ +'use strict'; + +const AppError = require('../errors/AppError'); +const logger = require('../logger'); +const config = require('../config'); + +/** + * Exempt internal probes and health check endpoints that cluster agents + * legitimately access over plain HTTP inside the private cluster network. + */ +function isExemptPath(path) { + if (!path) return false; + return ( + path === '/health' || + path.startsWith('/health/') || + path === '/metrics' + ); +} + +/** + * Determine the safe target hostname for HTTPS redirection. + * Never blindly reflects untrusted Host headers to prevent Host Header Injection. + */ +function resolveRedirectHost(req, httpsConfig) { + const primaryHost = httpsConfig.host; + const allowedHosts = httpsConfig.allowedHosts || []; + + const rawHost = req.get('host') || ''; + const hostWithoutPort = rawHost.split(':')[0].toLowerCase(); + + // If the host header is explicitly in the allowedHosts list, it is safe to use. + if (allowedHosts.length > 0) { + const isAllowed = allowedHosts.some((allowed) => { + const allowedLower = allowed.toLowerCase(); + return allowedLower === rawHost.toLowerCase() || allowedLower === hostWithoutPort; + }); + if (isAllowed) { + return rawHost; + } + } + + // Fall back to configured primary host, or first allowed host, or a safe default. + if (primaryHost) { + return primaryHost; + } + + if (allowedHosts.length > 0) { + return allowedHosts[0]; + } + + // Default fallback if no host is configured + return 'localhost'; +} + +/** + * Build Strict-Transport-Security header value based on configuration. + * + * NOTE ON PRELOAD: + * HSTS preload is strictly opt-in (`hstsPreload: true`). Submitting a domain + * to the browser HSTS preload list (hstspreload.org) hardcodes HTTPS into all + * major browsers. Removal is arduous and can take months, which risks breaking + * services if HTTPS configuration is ever rolled back. + */ +function buildHstsHeader(httpsConfig) { + const maxAge = httpsConfig.hstsMaxAge || 31536000; + let header = `max-age=${maxAge}`; + if (httpsConfig.hstsIncludeSubDomains) { + header += '; includeSubDomains'; + } + if (httpsConfig.hstsPreload) { + header += '; preload'; + } + return header; +} + +/** + * Middleware for HTTPS enforcement and HSTS header management (#468). + */ +function createHttpsEnforcementMiddleware(customConfig) { + return function httpsEnforcementMiddleware(req, res, next) { + const httpsConfig = customConfig || config.https || {}; + + // 1. Loop Guard and Misconfiguration Warning + // If a reverse proxy or CDN forwarded X-Forwarded-Proto: https, but Express + // reports req.secure === false, the Express `trust proxy` setting is missing or + // does not trust the proxy IP. This would cause an infinite redirect loop. + if (req.get('x-forwarded-proto')?.toLowerCase() === 'https' && !req.secure) { + logger.warn( + 'Potential HTTPS redirect loop detected: client provided X-Forwarded-Proto: https, ' + + 'but req.secure is false. Ensure Express "trust proxy" is properly configured for upstream proxies.', + { + ip: req.ip, + path: req.originalUrl || req.url, + remoteAddress: req.socket?.remoteAddress, + } + ); + } + + // 2. Strict-Transport-Security: Send ONLY on secure responses + if (req.secure) { + const hstsValue = buildHstsHeader(httpsConfig); + res.setHeader('Strict-Transport-Security', hstsValue); + } + + // 3. Check if HTTPS enforcement is disabled (e.g. local dev or edge SSL termination) + if (!httpsConfig.enforce) { + return next(); + } + + // 4. Secure requests pass through immediately + if (req.secure) { + return next(); + } + + // 5. Check exemptions for internal cluster probes (health & metrics) + const requestPath = req.path || req.originalUrl?.split('?')[0] || ''; + if (isExemptPath(requestPath)) { + return next(); + } + + // 6. State-changing requests over plain HTTP: Reject with 403 Forbidden + // Safe methods (GET, HEAD, OPTIONS) can be safely redirected. + // State-changing requests (POST, PUT, PATCH, DELETE) have ALREADY transmitted + // sensitive payload/credentials over cleartext HTTP. Redirecting them silently + // cannot remediate cleartext disclosure and may perpetuate insecure client habits. + const isSafeMethod = ['GET', 'HEAD', 'OPTIONS'].includes(req.method.toUpperCase()); + if (!isSafeMethod) { + logger.warn('Rejected state-changing request over plain HTTP', { + method: req.method, + path: req.originalUrl || req.url, + ip: req.ip, + }); + return next( + new AppError( + 'FORBIDDEN', + 'State-changing requests over plain HTTP are prohibited; HTTPS is required', + 403, + { + method: req.method, + protocol: req.protocol, + suggestion: 'Please re-issue the request over HTTPS', + } + ) + ); + } + + // 7. Safe methods over plain HTTP: 308 Permanent Redirect preserving method + const targetHost = resolveRedirectHost(req, httpsConfig); + const redirectUrl = `https://${targetHost}${req.originalUrl || req.url}`; + + logger.info('Redirecting plain HTTP request to HTTPS', { + originalUrl: req.originalUrl || req.url, + redirectUrl, + }); + + res.setHeader('Location', redirectUrl); + return res.status(308).end(); + }; +} + +module.exports = { + createHttpsEnforcementMiddleware, + httpsEnforcementMiddleware: createHttpsEnforcementMiddleware(), + isExemptPath, + resolveRedirectHost, + buildHstsHeader, +}; diff --git a/test/httpsEnforcement.test.js b/test/httpsEnforcement.test.js new file mode 100644 index 0000000..e15304a --- /dev/null +++ b/test/httpsEnforcement.test.js @@ -0,0 +1,225 @@ +'use strict'; + +const express = require('express'); +const request = require('supertest'); +const { + createHttpsEnforcementMiddleware, + buildHstsHeader, + resolveRedirectHost, +} = require('../src/middleware/httpsEnforcement'); +const { errorHandler } = require('../src/middleware/errorHandler'); + +describe('Issue #468: HTTPS Enforcement and HSTS Headers', () => { + function createApp(options = {}) { + const { + httpsConfig = { + enforce: true, + host: 'api.smartdrop.example.com', + allowedHosts: ['api.smartdrop.example.com', 'smartdrop.io'], + hstsMaxAge: 31536000, + hstsIncludeSubDomains: true, + hstsPreload: false, + }, + trustProxy = false, + } = options; + + const app = express(); + if (trustProxy !== undefined) { + app.set('trust proxy', trustProxy); + } + + app.use(createHttpsEnforcementMiddleware(httpsConfig)); + + // Sample endpoints + app.get('/health', (req, res) => res.status(200).json({ status: 'ok' })); + app.get('/health/live', (req, res) => res.status(200).json({ status: 'live' })); + app.get('/metrics', (req, res) => res.status(200).send('# metrics')); + app.get('/v1/prices', (req, res) => res.status(200).json({ price: 100 })); + app.post('/v1/airdrops', (req, res) => res.status(201).json({ created: true })); + + app.use(errorHandler); + return app; + } + + describe('1. Redirection and Method Preservation', () => { + it('redirects an HTTP GET request to the HTTPS URL with status 308 and Location', async () => { + const app = createApp(); + const res = await request(app) + .get('/v1/prices?asset=XLM') + .set('Host', 'api.smartdrop.example.com'); + + expect(res.status).toBe(308); + expect(res.headers.location).toBe('https://api.smartdrop.example.com/v1/prices?asset=XLM'); + }); + + it('rejects state-changing requests over plain HTTP with 403 Forbidden', async () => { + const app = createApp(); + const res = await request(app) + .post('/v1/airdrops') + .set('Host', 'api.smartdrop.example.com') + .send({ name: 'airdrop' }); + + expect(res.status).toBe(403); + expect(res.body.error).toBeDefined(); + expect(res.body.error.code).toBe('FORBIDDEN'); + expect(res.body.error.message).toContain('State-changing requests over plain HTTP are prohibited'); + }); + }); + + describe('2. Strict-Transport-Security (HSTS)', () => { + it('sends HSTS header on HTTPS requests with configured values', async () => { + const app = createApp({ + trustProxy: true, + httpsConfig: { + enforce: true, + hstsMaxAge: 31536000, + hstsIncludeSubDomains: true, + hstsPreload: false, + }, + }); + + const res = await request(app) + .get('/v1/prices') + .set('X-Forwarded-Proto', 'https'); + + expect(res.status).toBe(200); + expect(res.headers['strict-transport-security']).toBe('max-age=31536000; includeSubDomains'); + }); + + it('does NOT send HSTS header on plain HTTP requests', async () => { + const app = createApp({ + trustProxy: false, + httpsConfig: { enforce: false }, // Let HTTP pass through to check headers + }); + + const res = await request(app).get('/v1/prices'); + expect(res.status).toBe(200); + expect(res.headers['strict-transport-security']).toBeUndefined(); + }); + + it('only includes preload when strictly opted in', () => { + const headerWithoutPreload = buildHstsHeader({ + hstsMaxAge: 31536000, + hstsIncludeSubDomains: true, + hstsPreload: false, + }); + expect(headerWithoutPreload).toBe('max-age=31536000; includeSubDomains'); + + const headerWithPreload = buildHstsHeader({ + hstsMaxAge: 63072000, + hstsIncludeSubDomains: true, + hstsPreload: true, + }); + expect(headerWithPreload).toBe('max-age=63072000; includeSubDomains; preload'); + }); + + it('omits includeSubDomains when configured to false', () => { + const header = buildHstsHeader({ + hstsMaxAge: 86400, + hstsIncludeSubDomains: false, + hstsPreload: false, + }); + expect(header).toBe('max-age=86400'); + }); + }); + + describe('3. Trust Proxy & Secure Request Detection', () => { + it('ignores forged X-Forwarded-Proto from untrusted client (trust proxy: false)', async () => { + const app = createApp({ trustProxy: false }); + + // Client sends forged X-Forwarded-Proto: https, but trust proxy is disabled + const res = await request(app) + .get('/v1/prices') + .set('X-Forwarded-Proto', 'https') + .set('Host', 'api.smartdrop.example.com'); + + // Must NOT be trusted as HTTPS, so it redirects to HTTPS + expect(res.status).toBe(308); + expect(res.headers.location).toBe('https://api.smartdrop.example.com/v1/prices'); + }); + + it('honors forwarded protocol when trust proxy allows it', async () => { + const app = createApp({ trustProxy: true }); + + const res = await request(app) + .get('/v1/prices') + .set('X-Forwarded-Proto', 'https') + .set('Host', 'api.smartdrop.example.com'); + + // Trusted proxy says it is HTTPS, so it passes through + expect(res.status).toBe(200); + expect(res.body.price).toBe(100); + expect(res.headers['strict-transport-security']).toBeDefined(); + }); + }); + + describe('4. Host Header Injection Defense', () => { + it('does not reflect a hostile Host header in Location', async () => { + const app = createApp({ + httpsConfig: { + enforce: true, + host: 'api.smartdrop.example.com', + allowedHosts: ['api.smartdrop.example.com'], + hstsMaxAge: 31536000, + }, + }); + + const res = await request(app) + .get('/v1/prices') + .set('Host', 'attacker-controlled.com'); + + expect(res.status).toBe(308); + // Must redirect to configured public host, NEVER attacker-controlled.com + expect(res.headers.location).toBe('https://api.smartdrop.example.com/v1/prices'); + }); + + it('allows a Host header that matches the validated allow list', () => { + const req = { + get: (h) => (h.toLowerCase() === 'host' ? 'smartdrop.io' : null), + }; + const host = resolveRedirectHost(req, { + host: 'api.smartdrop.example.com', + allowedHosts: ['api.smartdrop.example.com', 'smartdrop.io'], + }); + expect(host).toBe('smartdrop.io'); + }); + }); + + describe('5. Health Check & Metrics Exemptions', () => { + it('exempts /health path from HTTPS redirection', async () => { + const app = createApp(); + const res = await request(app).get('/health'); + expect(res.status).toBe(200); + expect(res.body.status).toBe('ok'); + }); + + it('exempts /health/live path from HTTPS redirection', async () => { + const app = createApp(); + const res = await request(app).get('/health/live'); + expect(res.status).toBe(200); + expect(res.body.status).toBe('live'); + }); + + it('exempts /metrics path from HTTPS redirection', async () => { + const app = createApp(); + const res = await request(app).get('/metrics'); + expect(res.status).toBe(200); + expect(res.text).toBe('# metrics'); + }); + }); + + describe('6. Environment Variable Toggle', () => { + it('allows disabling enforcement via config (e.g. local development)', async () => { + const app = createApp({ + httpsConfig: { + enforce: false, + hstsMaxAge: 31536000, + }, + }); + + const res = await request(app).get('/v1/prices'); + expect(res.status).toBe(200); + expect(res.body.price).toBe(100); + }); + }); +}); From 3e471aadc589a0c85a334c0f75e4d6d11fd8a848 Mon Sep 17 00:00:00 2001 From: zaytherr Date: Sun, 18 Oct 2026 03:37:56 +0100 Subject: [PATCH 2/4] feat(auth): enforce session expiry (#469) --- .env.example | 11 + README.md | 4 + src/auth/index.js | 2 + src/auth/session.js | 382 +++++++++++++++++++++++++++++++++++ src/middleware/jwtAuth.js | 15 +- src/services/adminSession.js | 77 +++++-- test/sessionExpiry.test.js | 323 +++++++++++++++++++++++++++++ 7 files changed, 795 insertions(+), 19 deletions(-) create mode 100644 src/auth/session.js create mode 100644 test/sessionExpiry.test.js diff --git a/.env.example b/.env.example index 27d841b..03d3653 100644 --- a/.env.example +++ b/.env.example @@ -160,3 +160,14 @@ HSTS_INCLUDE_SUBDOMAINS=true # HSTS_PRELOAD: boolean. Default: false. Strictly opt-in; removal from browser preload lists takes months. HSTS_PRELOAD=false +# Session expiry enforcement (#469) +# SESSION_ABSOLUTE_LIFETIME_SECONDS: maximum session duration in seconds that activity can never extend. Default: 86400 (24h). +SESSION_ABSOLUTE_LIFETIME_SECONDS=86400 +# SESSION_IDLE_TIMEOUT_SECONDS: inactivity timeout in seconds that activity extends. Default: 1800 (30m). +SESSION_IDLE_TIMEOUT_SECONDS=1800 +# SESSION_ACTIVITY_THROTTLE_SECONDS: minimum interval in seconds between store writes on activity touch. Default: 60. +SESSION_ACTIVITY_THROTTLE_SECONDS=60 +# SESSION_CLOCK_SKEW_SECONDS: allowed clock-skew leeway in seconds for expiry comparisons. Default: 60. +SESSION_CLOCK_SKEW_SECONDS=60 + + diff --git a/README.md b/README.md index b2710ec..353a2a1 100644 --- a/README.md +++ b/README.md @@ -309,6 +309,10 @@ Each price source (`stellar_dex`, `coingecko`, `coinmarketcap`) gets one circuit | `HSTS_MAX_AGE_SECONDS` | HSTS max-age duration in seconds | 31536000 | No | | `HSTS_INCLUDE_SUBDOMAINS` | Include subdomains in HSTS header | true | No | | `HSTS_PRELOAD` | Opt-in to HSTS preloading | false | No | +| `SESSION_ABSOLUTE_LIFETIME_SECONDS` | Absolute maximum session lifetime in seconds (activity cannot extend) | 86400 (24h) | No | +| `SESSION_IDLE_TIMEOUT_SECONDS` | Inactivity timeout in seconds that activity extends | 1800 (30m) | No | +| `SESSION_ACTIVITY_THROTTLE_SECONDS` | Minimum interval in seconds between session store activity writes | 60 | No | +| `SESSION_CLOCK_SKEW_SECONDS` | Clock-skew leeway in seconds for expiry comparison | 60 | No | --- diff --git a/src/auth/index.js b/src/auth/index.js index 403b117..6c20d9c 100644 --- a/src/auth/index.js +++ b/src/auth/index.js @@ -2,8 +2,10 @@ const password = require('./password'); const authService = require('./authService'); +const session = require('./session'); module.exports = { ...password, ...authService, + ...session, }; diff --git a/src/auth/session.js b/src/auth/session.js new file mode 100644 index 0000000..fe74742 --- /dev/null +++ b/src/auth/session.js @@ -0,0 +1,382 @@ +'use strict'; + +/** + * Session management service with server-side expiry enforcement (Issue #469). + * + * Enforces dual-window expiry: + * 1. Absolute lifetime: fixed maximum session duration that activity can NEVER extend. + * 2. Idle timeout: inactivity window that extends on activity within the absolute lifetime. + * + * Guarantees: + * - All date arithmetic is performed in UTC. + * - Clock skew leeway is respected. + * - Store writes on activity updates are throttled to prevent write storms. + * - Expired or missing-expiry sessions/tokens return 401 UNAUTHORIZED. + * - Native store TTL + periodic unref'd cleanup timer. + * - Server-side invalidation on logout. + * - Legacy sessions without explicit expiry fields are supported by deriving from created_at. + * - Cookie options helper strictly mirrors remaining server-side session lifetime. + */ + +const crypto = require('crypto'); +const cache = require('../services/cache'); +const logger = require('../logger'); +const AppError = require('../errors/AppError'); + +function getEnvNumber(key, defaultValue) { + const val = process.env[key]; + if (val !== undefined && val !== '') { + const num = Number(val); + if (Number.isSafeInteger(num) && num > 0) return num; + } + return defaultValue; +} + +// Configuration with documented defaults +const ABSOLUTE_LIFETIME_SECONDS = getEnvNumber('SESSION_ABSOLUTE_LIFETIME_SECONDS', 24 * 60 * 60); // 24 hours +const IDLE_TIMEOUT_SECONDS = getEnvNumber('SESSION_IDLE_TIMEOUT_SECONDS', 30 * 60); // 30 minutes +const ACTIVITY_THROTTLE_SECONDS = getEnvNumber('SESSION_ACTIVITY_THROTTLE_SECONDS', 60); // 1 minute +const CLOCK_SKEW_SECONDS = getEnvNumber('SESSION_CLOCK_SKEW_SECONDS', 60); // 60 seconds + +const SESSIONS_ACTIVE_INDEX = 'sessions:active'; +let cleanupTimer = null; + +function sessionKey(id) { + return `session:${id}`; +} + +function generateSessionId() { + return `sess_${crypto.randomUUID().replace(/-/g, '')}`; +} + +function sanitizeSession(record) { + if (!record) return null; + return { ...record }; +} + +/** + * Derives expiry boundaries in UTC milliseconds. + * If record lacks absolute_expires_at or idle_expires_at (legacy session), + * derives them safely from created_at without requiring a database migration. + */ +function getSessionTimestamps(record) { + const createdAtMs = Date.parse(record.created_at) || Date.now(); + const lastActivityMs = Date.parse(record.last_activity_at || record.last_validated_at) || createdAtMs; + + const absoluteExpiresAtMs = record.absolute_expires_at + ? Date.parse(record.absolute_expires_at) + : createdAtMs + ABSOLUTE_LIFETIME_SECONDS * 1000; + + const idleExpiresAtMs = record.idle_expires_at + ? Date.parse(record.idle_expires_at) + : lastActivityMs + IDLE_TIMEOUT_SECONDS * 1000; + + return { + createdAtMs, + lastActivityMs, + absoluteExpiresAtMs, + idleExpiresAtMs, + }; +} + +/** + * Check if a session record has expired against either absolute or idle window. + */ +function isSessionExpired(record, nowMs = Date.now()) { + if (!record || record.active === false) return true; + + const { absoluteExpiresAtMs, idleExpiresAtMs } = getSessionTimestamps(record); + const leewayMs = CLOCK_SKEW_SECONDS * 1000; + + // 1. Check absolute lifetime: activity can NEVER extend this window + if (nowMs > absoluteExpiresAtMs + leewayMs) { + return true; + } + + // 2. Check idle timeout: exceeded inactivity window + if (nowMs > idleExpiresAtMs + leewayMs) { + return true; + } + + return false; +} + +/** + * Create a new server-side session. + */ +async function createSession(userId, metadata = {}) { + if (!userId) { + throw new Error('createSession: userId is required'); + } + + const now = new Date(); + const nowMs = now.getTime(); + const absoluteExpiresAtMs = nowMs + ABSOLUTE_LIFETIME_SECONDS * 1000; + const idleExpiresAtMs = nowMs + IDLE_TIMEOUT_SECONDS * 1000; + const effectiveExpiryMs = Math.min(absoluteExpiresAtMs, idleExpiresAtMs); + + const record = { + id: generateSessionId(), + user_id: userId, + active: true, + created_at: now.toISOString(), + last_activity_at: now.toISOString(), + absolute_expires_at: new Date(absoluteExpiresAtMs).toISOString(), + idle_expires_at: new Date(idleExpiresAtMs).toISOString(), + ip_address: metadata.ipAddress || null, + user_agent: metadata.userAgent || null, + }; + + const ttlSeconds = Math.max(1, Math.ceil((absoluteExpiresAtMs - nowMs) / 1000)); + await cache.set(sessionKey(record.id), record, ttlSeconds); + + try { + const redis = cache.getClient(); + if (redis && typeof redis.zadd === 'function') { + await redis.zadd(SESSIONS_ACTIVE_INDEX, effectiveExpiryMs, record.id); + } + } catch (err) { + logger.warn('Failed to add session to active index', { error: err.message }); + } + + return sanitizeSession(record); +} + +/** + * Validate session and update last-activity with throttling. + * + * If session is expired or invalid: returns null. + * If activity write is throttled: returns session without Redis write. + * If throttle window has elapsed: updates last_activity_at and extends idle timeout + * (up to the fixed absolute lifetime). + */ +async function validateSession(sessionId, { touch = true, now = new Date() } = {}) { + if (!sessionId) return null; + + const record = await cache.get(sessionKey(sessionId)); + if (!record) return null; + + const nowMs = now.getTime(); + if (isSessionExpired(record, nowMs)) { + // Lazily clean up expired session + await invalidateSession(sessionId); + return null; + } + + if (!touch) { + return sanitizeSession(record); + } + + const { lastActivityMs, absoluteExpiresAtMs } = getSessionTimestamps(record); + const throttleMs = ACTIVITY_THROTTLE_SECONDS * 1000; + + // Throttling: If last write was within throttle window, DO NOT write to store + if (nowMs - lastActivityMs < throttleMs) { + return sanitizeSession(record); + } + + // Extend idle expiry, but NEVER exceed the absolute lifetime + const newIdleExpiresAtMs = Math.min(nowMs + IDLE_TIMEOUT_SECONDS * 1000, absoluteExpiresAtMs); + const remainingTtlSeconds = Math.max(1, Math.ceil((newIdleExpiresAtMs - nowMs) / 1000)); + + const updatedRecord = { + ...record, + last_activity_at: now.toISOString(), + idle_expires_at: new Date(newIdleExpiresAtMs).toISOString(), + // Preserve or derive absolute_expires_at for legacy sessions + absolute_expires_at: record.absolute_expires_at || new Date(absoluteExpiresAtMs).toISOString(), + }; + + await cache.set(sessionKey(sessionId), updatedRecord, remainingTtlSeconds); + + try { + const redis = cache.getClient(); + if (redis && typeof redis.zadd === 'function') { + await redis.zadd(SESSIONS_ACTIVE_INDEX, newIdleExpiresAtMs, sessionId); + } + } catch (err) { + logger.warn('Failed to update session active index score', { error: err.message }); + } + + return sanitizeSession(updatedRecord); +} + +/** + * Invalidate a session server-side (logout / revocation). + */ +async function invalidateSession(sessionId) { + if (!sessionId) return false; + + try { + await cache.del(sessionKey(sessionId)); + const redis = cache.getClient(); + if (redis && typeof redis.zrem === 'function') { + await redis.zrem(SESSIONS_ACTIVE_INDEX, sessionId); + } + return true; + } catch (err) { + logger.error('Failed to invalidate session', { sessionId, error: err.message }); + return false; + } +} + +/** + * Scheduled cleanup of expired sessions from active index. + */ +async function cleanupExpiredSessions({ limit = 500, nowMs = Date.now() } = {}) { + try { + const redis = cache.getClient(); + if (!redis || typeof redis.zrangebyscore !== 'function') { + return { cleaned: 0, checked: 0 }; + } + + const expiredIds = await redis.zrangebyscore( + SESSIONS_ACTIVE_INDEX, + 0, + nowMs, + 'LIMIT', + 0, + limit + ); + + if (!expiredIds || expiredIds.length === 0) { + return { cleaned: 0, checked: 0 }; + } + + const pipeline = redis.multi ? redis.multi() : null; + if (pipeline) { + expiredIds.forEach((id) => pipeline.zrem(SESSIONS_ACTIVE_INDEX, id)); + await pipeline.exec(); + } else { + await Promise.all(expiredIds.map((id) => redis.zrem(SESSIONS_ACTIVE_INDEX, id))); + } + + await Promise.all(expiredIds.map((id) => cache.del(sessionKey(id)))); + return { cleaned: expiredIds.length, checked: expiredIds.length }; + } catch (err) { + logger.error('cleanupExpiredSessions error', { error: err.message }); + return { cleaned: 0, checked: 0 }; + } +} + +/** + * Start background periodic cleanup with an unref'd timer (no listener leaks, no hanging process). + */ +function startCleanupTimer(intervalMs = 15 * 60 * 1000) { + if (cleanupTimer) return; + cleanupTimer = setInterval(() => { + cleanupExpiredSessions().catch((err) => { + logger.warn('Periodic session cleanup encountered error', { error: err.message }); + }); + }, intervalMs); + + if (typeof cleanupTimer.unref === 'function') { + cleanupTimer.unref(); + } +} + +function stopCleanupTimer() { + if (cleanupTimer) { + clearInterval(cleanupTimer); + cleanupTimer = null; + } +} + +/** + * Helper to compute secure cookie options aligned with server-side session expiry (#496). + */ +function getSessionCookieOptions(session, options = {}) { + const isSecure = options.isSecure !== undefined + ? options.isSecure + : process.env.NODE_ENV === 'production'; + + const nowMs = Date.now(); + let maxAgeMs = IDLE_TIMEOUT_SECONDS * 1000; + + if (session) { + const { absoluteExpiresAtMs, idleExpiresAtMs } = getSessionTimestamps(session); + const effectiveExpiryMs = Math.min(absoluteExpiresAtMs, idleExpiresAtMs); + maxAgeMs = Math.max(0, effectiveExpiryMs - nowMs); + } + + return { + httpOnly: true, + secure: isSecure, + sameSite: 'lax', + path: '/', + maxAge: maxAgeMs, + }; +} + +/** + * Express middleware for authenticating requests via session. + */ +function sessionAuth(options = {}) { + const { required = true } = options; + + return async (req, res, next) => { + const rawCookie = req.headers.cookie || ''; + let sessionId = null; + + // Extract session from cookie: session_id=... + const cookieMatch = rawCookie.match(/(?:^|;\s*)session_id=([^;]+)/); + if (cookieMatch) { + sessionId = decodeURIComponent(cookieMatch[1]); + } else if (req.get('x-session-id')) { + sessionId = req.get('x-session-id'); + } + + if (!sessionId) { + if (required) { + return next(new AppError('UNAUTHORIZED', 'Invalid credentials', 401)); + } + return next(); + } + + try { + // Validate session; pass touch: false so a request that subsequently fails + // will NOT have extended session activity + const session = await validateSession(sessionId, { touch: false }); + if (!session) { + return next(new AppError('UNAUTHORIZED', 'Invalid credentials', 401)); + } + + req.session = session; + req.user = { id: session.user_id }; + + // Touch session only when response finishes successfully without auth failure + res.on('finish', () => { + if (res.statusCode < 400) { + validateSession(sessionId, { touch: true }).catch((err) => { + logger.warn('Failed to touch session on request finish', { + sessionId, + error: err.message, + }); + }); + } + }); + + return next(); + } catch (err) { + logger.error('Session authentication error', { error: err.message }); + return next(new AppError('UNAUTHORIZED', 'Invalid credentials', 401)); + } + }; +} + +module.exports = { + createSession, + validateSession, + invalidateSession, + cleanupExpiredSessions, + startCleanupTimer, + stopCleanupTimer, + getSessionCookieOptions, + sessionAuth, + isSessionExpired, + getSessionTimestamps, + ABSOLUTE_LIFETIME_SECONDS, + IDLE_TIMEOUT_SECONDS, + ACTIVITY_THROTTLE_SECONDS, + CLOCK_SKEW_SECONDS, +}; diff --git a/src/middleware/jwtAuth.js b/src/middleware/jwtAuth.js index ed35847..737b419 100644 --- a/src/middleware/jwtAuth.js +++ b/src/middleware/jwtAuth.js @@ -89,12 +89,17 @@ function verifyJwt(token, secret, options = {}) { throw new AppError('UNAUTHORIZED', 'Invalid JWT payload', 401); } - // Expiration check + // Expiration check (#469) + // A token that carries no expiry must be rejected, not treated as eternal. + const CLOCK_SKEW_LEEWAY_SECONDS = parseInt(process.env.AUTH_CLOCK_SKEW_SECONDS, 10) || 60; const now = Math.floor(Date.now() / 1000); - if (payload.exp !== undefined && typeof payload.exp === 'number') { - if (payload.exp < now) { - throw new AppError('UNAUTHORIZED', 'JWT has expired', 401); - } + if (payload.exp === undefined || typeof payload.exp !== 'number') { + logger.warn('JWT rejected: missing or invalid exp claim'); + throw new AppError('UNAUTHORIZED', 'Invalid token', 401); + } + if (payload.exp + CLOCK_SKEW_LEEWAY_SECONDS < now) { + logger.warn('JWT rejected: token expired', { exp: payload.exp, now }); + throw new AppError('UNAUTHORIZED', 'Invalid token', 401); } // Not-before check diff --git a/src/services/adminSession.js b/src/services/adminSession.js index e6c56de..3911dac 100644 --- a/src/services/adminSession.js +++ b/src/services/adminSession.js @@ -35,6 +35,14 @@ const logger = require('../logger'); const SESSION_TTL_SECONDS = parseInt(process.env.ADMIN_SESSION_TTL_SECONDS, 10) || 12 * 60 * 60; +const SESSION_ABSOLUTE_LIFETIME_SECONDS = + parseInt(process.env.SESSION_ABSOLUTE_LIFETIME_SECONDS || process.env.ADMIN_SESSION_TTL_SECONDS, 10) || 12 * 60 * 60; +const SESSION_IDLE_TIMEOUT_SECONDS = + parseInt(process.env.SESSION_IDLE_TIMEOUT_SECONDS, 10) || 30 * 60; +const SESSION_ACTIVITY_THROTTLE_SECONDS = + parseInt(process.env.SESSION_ACTIVITY_THROTTLE_SECONDS, 10) || 60; +const CLOCK_SKEW_SECONDS = + parseInt(process.env.SESSION_CLOCK_SKEW_SECONDS, 10) || 60; const ACTIVE_INDEX_KEY = 'admin_sessions:active'; const ADMISNS_KEY = 'admins:ids'; // Bound a single cleanup pass so a huge backlog of expired sessions cannot @@ -67,14 +75,33 @@ function sanitize(record) { function isExpired(record, nowMs = Date.now()) { if (!record) return true; if (record.active === false) return true; - const expiresAt = Date.parse(record.expires_at); - return Number.isNaN(expiresAt) || expiresAt <= nowMs; + + const createdAtMs = Date.parse(record.created_at) || nowMs; + const lastActivityMs = Date.parse(record.last_activity_at || record.last_validated_at) || createdAtMs; + + const absoluteExpiresAtMs = record.absolute_expires_at + ? Date.parse(record.absolute_expires_at) + : (record.expires_at ? Date.parse(record.expires_at) : createdAtMs + SESSION_ABSOLUTE_LIFETIME_SECONDS * 1000); + + const idleExpiresAtMs = record.idle_expires_at + ? Date.parse(record.idle_expires_at) + : lastActivityMs + SESSION_IDLE_TIMEOUT_SECONDS * 1000; + + const leewayMs = CLOCK_SKEW_SECONDS * 1000; + if (Number.isNaN(absoluteExpiresAtMs) || nowMs > absoluteExpiresAtMs + leewayMs) { + return true; + } + if (nowMs > idleExpiresAtMs + leewayMs) { + return true; + } + return false; } /** * #429 — the session record is written with a TTL (SETEX via cache.set) and * both index keys get an EXPIRE as well, so session data evicts itself even * if it is never accessed again. + * #469 — enforces dual-window expiry (absolute lifetime + idle timeout). */ async function createSession(adminId, options = {}) { if (!adminId || typeof adminId !== 'string') { @@ -82,28 +109,33 @@ async function createSession(adminId, options = {}) { } const now = new Date(); const nowMs = now.getTime(); - const expiresAtMs = nowMs + SESSION_TTL_SECONDS * 1000; + const absoluteExpiresAtMs = nowMs + SESSION_ABSOLUTE_LIFETIME_SECONDS * 1000; + const idleExpiresAtMs = nowMs + SESSION_IDLE_TIMEOUT_SECONDS * 1000; + const effectiveExpiryMs = Math.min(absoluteExpiresAtMs, idleExpiresAtMs); + const ttlSeconds = Math.max(1, Math.ceil((absoluteExpiresAtMs - nowMs) / 1000)); + const record = { id: generateSessionId(), admin_id: adminId, active: true, created_at: now.toISOString(), - expires_at: new Date(expiresAtMs).toISOString(), + expires_at: new Date(absoluteExpiresAtMs).toISOString(), + absolute_expires_at: new Date(absoluteExpiresAtMs).toISOString(), + idle_expires_at: new Date(idleExpiresAtMs).toISOString(), last_validated_at: now.toISOString(), + last_activity_at: now.toISOString(), ip_address: options.ipAddress || null, user_agent: options.userAgent || null, audit_history: [], }; const redis = cache.getClient(); - // Session record carries the TTL; the per-admin list and global index are - // only pointers, so they get the same TTL to avoid outliving the data. - await cache.set(sessionKey(record.id), record, SESSION_TTL_SECONDS); + await cache.set(sessionKey(record.id), record, ttlSeconds); const multi = redis.multi(); - multi.zadd(adminSessionsKey(adminId), expiresAtMs, record.id); - multi.expire(adminSessionsKey(adminId), SESSION_TTL_SECONDS); - multi.zadd(ACTIVE_INDEX_KEY, expiresAtMs, record.id); - multi.expire(ACTIVE_INDEX_KEY, SESSION_TTL_SECONDS); + multi.zadd(adminSessionsKey(adminId), effectiveExpiryMs, record.id); + multi.expire(adminSessionsKey(adminId), ttlSeconds); + multi.zadd(ACTIVE_INDEX_KEY, effectiveExpiryMs, record.id); + multi.expire(ACTIVE_INDEX_KEY, ttlSeconds); multi.sadd(ADMINS_KEY, adminId); await multi.exec(); @@ -251,13 +283,30 @@ async function validateSession(sessionId) { return null; } - // Refresh in place and bump the TTL so active sessions don't expire while - // in use (#429). Single write, no re-read. + const createdAtMs = Date.parse(record.created_at) || nowMs; + const lastActivityMs = Date.parse(record.last_activity_at || record.last_validated_at) || createdAtMs; + const absoluteExpiresAtMs = record.absolute_expires_at + ? Date.parse(record.absolute_expires_at) + : (record.expires_at ? Date.parse(record.expires_at) : createdAtMs + SESSION_ABSOLUTE_LIFETIME_SECONDS * 1000); + + // Throttled write: only write to store if interval elapsed + const throttleMs = SESSION_ACTIVITY_THROTTLE_SECONDS * 1000; + if (nowMs - lastActivityMs < throttleMs) { + return sanitize(record); + } + + // Extend idle window, bounded by absolute lifetime + const newIdleExpiresAtMs = Math.min(nowMs + SESSION_IDLE_TIMEOUT_SECONDS * 1000, absoluteExpiresAtMs); + const remainingTtlSeconds = Math.max(1, Math.ceil((newIdleExpiresAtMs - nowMs) / 1000)); + const refreshed = { ...record, last_validated_at: new Date(nowMs).toISOString(), + last_activity_at: new Date(nowMs).toISOString(), + idle_expires_at: new Date(newIdleExpiresAtMs).toISOString(), + absolute_expires_at: record.absolute_expires_at || new Date(absoluteExpiresAtMs).toISOString(), }; - await cache.set(sessionKey(sessionId), refreshed, SESSION_TTL_SECONDS); + await cache.set(sessionKey(sessionId), refreshed, remainingTtlSeconds); return sanitize(refreshed); } diff --git a/test/sessionExpiry.test.js b/test/sessionExpiry.test.js new file mode 100644 index 0000000..612d370 --- /dev/null +++ b/test/sessionExpiry.test.js @@ -0,0 +1,323 @@ +'use strict'; + +const crypto = require('crypto'); +const express = require('express'); +const request = require('supertest'); + +const mockStore = new Map(); +const mockSortedSets = new Map(); + +jest.mock('../src/services/cache', () => { + return { + getClient: () => ({ + zadd: jest.fn(async (key, score, member) => { + if (!mockSortedSets.has(key)) mockSortedSets.set(key, new Map()); + mockSortedSets.get(key).set(member, score); + }), + zrem: jest.fn(async (key, member) => { + mockSortedSets.get(key)?.delete(member); + }), + zrangebyscore: jest.fn(async (key, min, max) => { + const sortedSet = mockSortedSets.get(key); + if (!sortedSet) return []; + const minVal = min === '-inf' ? -Infinity : Number(min); + const maxVal = max === '+inf' ? Infinity : Number(max); + const results = []; + for (const [member, score] of sortedSet.entries()) { + if (score >= minVal && score <= maxVal) { + results.push(member); + } + } + return results; + }), + multi: jest.fn(() => ({ + zrem: jest.fn(), + exec: jest.fn(async () => []), + })), + }), + get: jest.fn(async (key) => { + const v = mockStore.get(key); + return v !== undefined ? JSON.parse(JSON.stringify(v)) : null; + }), + set: jest.fn(async (key, value) => { + mockStore.set(key, JSON.parse(JSON.stringify(value))); + }), + del: jest.fn(async (key) => { + mockStore.delete(key); + }), + }; +}); + +const cache = require('../src/services/cache'); +const { + createSession, + validateSession, + invalidateSession, + cleanupExpiredSessions, + sessionAuth, + getSessionCookieOptions, + isSessionExpired, + ABSOLUTE_LIFETIME_SECONDS, + IDLE_TIMEOUT_SECONDS, + ACTIVITY_THROTTLE_SECONDS, +} = require('../src/auth/session'); +const { verifyJwt } = require('../src/middleware/jwtAuth'); +const { errorHandler } = require('../src/middleware/errorHandler'); + +describe('Issue #469: Session Expiry Enforcement', () => { + beforeEach(() => { + mockStore.clear(); + mockSortedSets.clear(); + jest.clearAllMocks(); + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + describe('1. Dual-Window Session Lifetimes', () => { + it('accepts a session that is within both the absolute and idle windows', async () => { + const now = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(now); + + const session = await createSession('user-123'); + const validated = await validateSession(session.id); + + expect(validated).not.toBeNull(); + expect(validated.id).toBe(session.id); + expect(validated.user_id).toBe('user-123'); + }); + + it('rejects a session past the absolute lifetime even if it was active a moment ago', async () => { + const startTime = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(startTime); + + const session = await createSession('user-123'); + + // Fast forward past absolute lifetime (default 24h = 86400s) + leeway + const pastAbsolute = new Date(startTime.getTime() + (ABSOLUTE_LIFETIME_SECONDS + 120) * 1000); + jest.setSystemTime(pastAbsolute); + + // Even if last_activity_at was 1 second ago: + const activeRecord = { + ...session, + last_activity_at: new Date(pastAbsolute.getTime() - 1000).toISOString(), + }; + await cache.set(`session:${session.id}`, activeRecord); + + const validated = await validateSession(session.id); + expect(validated).toBeNull(); + }); + + it('rejects a session past the idle timeout window', async () => { + const startTime = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(startTime); + + const session = await createSession('user-123'); + + // Advance past idle timeout (default 30m = 1800s) + clock skew leeway (60s) + const pastIdle = new Date(startTime.getTime() + (IDLE_TIMEOUT_SECONDS + 120) * 1000); + jest.setSystemTime(pastIdle); + + const validated = await validateSession(session.id); + expect(validated).toBeNull(); + }); + + it('extends the idle expiry on activity within the idle window, but never exceeds the absolute lifetime', async () => { + const startTime = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(startTime); + + const session = await createSession('user-123'); + const absoluteExpiryMs = Date.parse(session.absolute_expires_at); + + // Advance 10 minutes (within idle window and past throttle window) + const t1 = new Date(startTime.getTime() + 10 * 60 * 1000); + jest.setSystemTime(t1); + + const touched = await validateSession(session.id); + expect(touched).not.toBeNull(); + const idleExpiry1 = Date.parse(touched.idle_expires_at); + expect(idleExpiry1).toBe(t1.getTime() + IDLE_TIMEOUT_SECONDS * 1000); + + // Advance to near absolute expiration (absolute - 5 minutes) + const nearEnd = new Date(absoluteExpiryMs - 5 * 60 * 1000); + jest.setSystemTime(nearEnd); + + const endTouched = await validateSession(session.id); + expect(endTouched).not.toBeNull(); + // Idle expiry must be capped at absolute lifetime, never extend past it + const finalIdleExpiry = Date.parse(endTouched.idle_expires_at); + expect(finalIdleExpiry).toBeLessThanOrEqual(absoluteExpiryMs); + }); + }); + + describe('2. JWT Missing Expiry Rejection', () => { + const secret = 'super-secret-key-32-bytes-long!'; + + function createJwt(payload, alg = 'HS256') { + const header = { alg, typ: 'JWT' }; + const hB64 = Buffer.from(JSON.stringify(header)).toString('base64url'); + const pB64 = Buffer.from(JSON.stringify(payload)).toString('base64url'); + const data = `${hB64}.${pB64}`; + const sig = crypto.createHmac('sha256', secret).update(data).digest('base64url'); + return `${data}.${sig}`; + } + + it('rejects a JWT token without an exp claim', () => { + const tokenWithoutExp = createJwt({ sub: 'user-1', name: 'Alice' }); + expect(() => { + verifyJwt(tokenWithoutExp, secret); + }).toThrow('Invalid token'); + }); + + it('rejects an expired JWT token', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const expiredToken = createJwt({ sub: 'user-1', exp: nowSeconds - 300 }); // expired 5 mins ago + expect(() => { + verifyJwt(expiredToken, secret); + }).toThrow('Invalid token'); + }); + + it('accepts a valid JWT with future exp', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const validToken = createJwt({ sub: 'user-1', exp: nowSeconds + 3600 }); + const decoded = verifyJwt(validToken, secret); + expect(decoded.sub).toBe('user-1'); + }); + }); + + describe('3. Activity Write Throttling and Failure Safety', () => { + it('throttles last-activity writes so store is not updated on every request', async () => { + const startTime = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(startTime); + + const session = await createSession('user-123'); + cache.set.mockClear(); + + // Rapid requests within throttle interval (e.g. 10s < ACTIVITY_THROTTLE_SECONDS 60s) + jest.setSystemTime(new Date(startTime.getTime() + 10 * 1000)); + await validateSession(session.id); + expect(cache.set).not.toHaveBeenCalled(); + + jest.setSystemTime(new Date(startTime.getTime() + 30 * 1000)); + await validateSession(session.id); + expect(cache.set).not.toHaveBeenCalled(); + + // Request after throttle interval (e.g. 70s > 60s) + jest.setSystemTime(new Date(startTime.getTime() + 70 * 1000)); + await validateSession(session.id); + expect(cache.set).toHaveBeenCalledTimes(1); + }); + + it('does not extend the session if the request fails with an error', async () => { + const startTime = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(startTime); + + const session = await createSession('user-123'); + cache.set.mockClear(); + + const app = express(); + app.use(sessionAuth({ required: true })); + app.get('/failing-endpoint', (req, res, next) => { + const err = new Error('Database error'); + err.statusCode = 500; + next(err); + }); + app.use(errorHandler); + + // Advance time past throttle threshold + jest.setSystemTime(new Date(startTime.getTime() + 120 * 1000)); + + const res = await request(app) + .get('/failing-endpoint') + .set('Cookie', `session_id=${session.id}`); + + expect(res.status).toBe(500); + // Session store write must NOT occur on a failed request + expect(cache.set).not.toHaveBeenCalled(); + }); + }); + + describe('4. Invalidation and Cleanup', () => { + it('invalidates the session on logout', async () => { + const session = await createSession('user-123'); + const ok = await invalidateSession(session.id); + expect(ok).toBe(true); + + const retrieved = await validateSession(session.id); + expect(retrieved).toBeNull(); + }); + + it('cleans up expired entries from the active index and store', async () => { + const now = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(now); + + const session = await createSession('user-123'); + + // Fast forward past expiry + const future = new Date(now.getTime() + 100000 * 1000); + jest.setSystemTime(future); + + const result = await cleanupExpiredSessions({ nowMs: future.getTime() }); + expect(result.checked).toBeGreaterThanOrEqual(1); + }); + }); + + describe('5. Legacy Session Compatibility', () => { + it('gracefully handles legacy sessions without expiry fields by deriving from created_at', async () => { + const now = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(now); + + const legacySession = { + id: 'sess_legacy123', + user_id: 'legacy-user', + active: true, + created_at: new Date(now.getTime() - 10 * 60 * 1000).toISOString(), // 10 minutes ago + // Notice: absolute_expires_at and idle_expires_at are absent! + }; + await cache.set(`session:${legacySession.id}`, legacySession); + + const validated = await validateSession(legacySession.id); + expect(validated).not.toBeNull(); + expect(validated.absolute_expires_at).toBeDefined(); + expect(validated.idle_expires_at).toBeDefined(); + }); + + it('rejects an old legacy session whose created_at is past the absolute lifetime', async () => { + const now = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(now); + + const oldLegacySession = { + id: 'sess_oldlegacy123', + user_id: 'legacy-user', + active: true, + created_at: new Date(now.getTime() - (ABSOLUTE_LIFETIME_SECONDS + 3600) * 1000).toISOString(), + }; + await cache.set(`session:${oldLegacySession.id}`, oldLegacySession); + + const validated = await validateSession(oldLegacySession.id); + expect(validated).toBeNull(); + }); + }); + + describe('6. Cookie Lifetime Alignment (#496)', () => { + it('sets cookie maxAge strictly equal to remaining server-side session lifetime', () => { + const now = new Date('2026-10-18T10:00:00.000Z'); + jest.setSystemTime(now); + + const session = { + created_at: now.toISOString(), + last_activity_at: now.toISOString(), + absolute_expires_at: new Date(now.getTime() + 86400 * 1000).toISOString(), + idle_expires_at: new Date(now.getTime() + 1800 * 1000).toISOString(), + }; + + const options = getSessionCookieOptions(session, { isSecure: true }); + expect(options.httpOnly).toBe(true); + expect(options.secure).toBe(true); + expect(options.sameSite).toBe('lax'); + expect(options.maxAge).toBe(1800 * 1000); + }); + }); +}); From 5c79c26b06260424a910d3c7f15906ec159ffdb4 Mon Sep 17 00:00:00 2001 From: zaytherr Date: Sun, 18 Oct 2026 03:41:03 +0100 Subject: [PATCH 3/4] feat(middleware): add CSRF protection (#465) --- src/index.js | 9 ++ src/middleware/cors.js | 2 +- src/middleware/csrf.js | 315 +++++++++++++++++++++++++++++++++--- test/csrfProtection.test.js | 281 ++++++++++++++++++++++++++++++++ 4 files changed, 585 insertions(+), 22 deletions(-) create mode 100644 test/csrfProtection.test.js diff --git a/src/index.js b/src/index.js index 286cae5..79ce54b 100644 --- a/src/index.js +++ b/src/index.js @@ -33,6 +33,10 @@ const { attachApiKey, auditApiKeyUsage, } = require("./middleware/auth"); +const { + csrfProtectionMiddleware, + csrfTokenHandler, +} = require("./middleware/csrf"); const { errorHandler, notFoundHandler } = require("./middleware/errorHandler"); const { checkDatabase } = require("./services/dbHealth"); const pricesRouter = require("./routes/prices"); @@ -295,9 +299,14 @@ app.use(globalRateLimit); app.use(requestValidationMiddleware); app.use("/static", createStaticAssetsMiddleware(config.staticAssets)); +// #465 — Global CSRF protection for state-changing requests and CSRF token endpoint +app.get("/csrf-token", csrfTokenHandler); +app.use(csrfProtectionMiddleware); + // ==================== API ROUTING & VERSIONING (#460) ==================== // Build the core v1 API router. const apiV1Router = express.Router(); +apiV1Router.get("/csrf-token", csrfTokenHandler); apiV1Router.use(attachApiKey()); apiV1Router.use(auditApiKeyUsage()); apiV1Router.use(apiKeyLimit); diff --git a/src/middleware/cors.js b/src/middleware/cors.js index 4597dd3..105f382 100644 --- a/src/middleware/cors.js +++ b/src/middleware/cors.js @@ -15,7 +15,7 @@ function buildCorsMiddleware(allowedOrigins) { }, credentials: true, methods: ['GET', 'POST', 'DELETE', 'PATCH', 'OPTIONS'], - allowedHeaders: ['Content-Type', 'Authorization', 'X-Requested-With'], + allowedHeaders: ['Content-Type', 'Authorization', 'X-Requested-With', 'X-CSRF-Token'], // Issue #344: preflight (OPTIONS) responses carry Access-Control-Max-Age // so browsers can cache the result instead of re-sending a preflight for // every cross-origin POST. Window defaults to 24h and is tunable via diff --git a/src/middleware/csrf.js b/src/middleware/csrf.js index 5ee84a0..d8bc1e8 100644 --- a/src/middleware/csrf.js +++ b/src/middleware/csrf.js @@ -1,34 +1,307 @@ -const AppError = require("../errors/AppError"); +'use strict'; /** - * CSRF defense-in-depth for state-changing webhook endpoints (#320). + * Global CSRF Protection Middleware (Issue #465). * - * `buildCorsMiddleware` already rejects cross-origin requests whose Origin - * header doesn't match `corsAllowedOrigins` — but it treats a request with - * NO Origin header as automatically allowed (`if (!origin || ...)`), and a - * plain HTML `
` submission can omit Origin in some conditions and - * carries no JSON body / custom headers at all. Requiring this header - * closes that gap: a real browser CSRF (no attacker-controlled JS, just a - * form or an / tag) can never set a custom header, so a request - * missing it is never a cross-site form submission — regardless of what - * Origin/Referer it did or didn't send. + * Denies state-changing requests by default unless valid CSRF credentials are provided. * - * This does not replace `requireApiKey` — a stolen API key still lets an - * attacker call the API directly. It specifically closes the "victim's - * authenticated browser session becomes an unwitting proxy" vector the - * issue describes. + * Protection Strategy: + * 1. Safe methods (GET, HEAD, OPTIONS) are exempt. + * 2. Requests authenticated by Authorization header or API key are exempt (ambient browser cookies are absent). + * 3. Declared webhook endpoints with signature verification are exempt. + * 4. Method override headers and parameters (_method, X-HTTP-Method-Override, etc.) are strictly rejected. + * 5. State-changing requests must pass an Origin/Referer check against configured allowed origins. + * 6. Per-session cryptographically signed token (HMAC-SHA256) sent via X-CSRF-Token header. + * 7. Token is validated using constant-time timingSafeEqual to defeat timing attacks. + * 8. Never accepts CSRF tokens from URL query parameters. + * 9. Standard 403 FORBIDDEN error response on failure; tokens and cookies are never logged. + */ + +const crypto = require('crypto'); +const AppError = require('../errors/AppError'); +const logger = require('../logger'); +const config = require('../config'); + +const CSRF_SECRET = process.env.CSRF_SECRET || 'smartdrop-csrf-token-hmac-secret-key-32b'; + +const METHOD_OVERRIDE_HEADERS = [ + 'x-http-method-override', + 'x-method-override', + 'x-http-method', +]; + +const SAFE_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']); + +const DEFAULT_EXEMPT_PATHS = [ + '/health', + '/metrics', + '/api-docs', + '/csrf-token', + '/api/v1/csrf-token', + '/v1/csrf-token', +]; + +/** + * Generate a cryptographically secure CSRF token bound to a session ID. + */ +function generateCsrfToken(sessionId, secret = CSRF_SECRET) { + const boundId = sessionId || 'anonymous'; + const salt = crypto.randomBytes(16).toString('hex'); + const hmac = crypto + .createHmac('sha256', secret) + .update(`${boundId}:${salt}`) + .digest('hex'); + return `${salt}.${hmac}`; +} + +/** + * Validate a CSRF token in constant time. + */ +function verifyCsrfToken(token, sessionId, secret = CSRF_SECRET) { + if (!token || typeof token !== 'string') return false; + + const parts = token.split('.'); + if (parts.length !== 2) return false; + + const [salt, receivedHmac] = parts; + if (!salt || !receivedHmac) return false; + + const boundId = sessionId || 'anonymous'; + const expectedHmac = crypto + .createHmac('sha256', secret) + .update(`${boundId}:${salt}`) + .digest('hex'); + + const receivedBuf = Buffer.from(receivedHmac, 'utf8'); + const expectedBuf = Buffer.from(expectedHmac, 'utf8'); + + if (receivedBuf.length !== expectedBuf.length) return false; + return crypto.timingSafeEqual(receivedBuf, expectedBuf); +} + +/** + * Check if the request attempts a method-override bypass. + */ +function hasMethodOverride(req) { + for (const header of METHOD_OVERRIDE_HEADERS) { + if (req.headers[header]) return true; + } + if (req.query && req.query._method) return true; + if (req.body && typeof req.body === 'object' && req.body._method) return true; + return false; +} + +/** + * Verify Origin or Referer against allowed origins list. + */ +function isAllowedOrigin(req, allowedOrigins) { + const origin = req.get('origin'); + if (origin) { + try { + const originBase = new URL(origin).origin; + return allowedOrigins.some((allowed) => { + try { + return new URL(allowed).origin === originBase; + } catch { + return allowed.trim() === origin.trim(); + } + }); + } catch { + return false; + } + } + + // Fallback to Referer + const referer = req.get('referer'); + if (referer) { + try { + const refererBase = new URL(referer).origin; + return allowedOrigins.some((allowed) => { + try { + return new URL(allowed).origin === refererBase; + } catch { + return allowed.trim() === refererBase.trim(); + } + }); + } catch { + return false; + } + } + + // Both Origin and Referer are absent + return false; +} + +/** + * Extracts session ID from request (cookie or request attachment). + */ +function extractSessionId(req) { + if (req.session?.id) return req.session.id; + + const rawCookie = req.headers.cookie || ''; + const match = rawCookie.match(/(?:^|;\s*)session_id=([^;]+)/); + if (match) { + return decodeURIComponent(match[1]); + } + return null; +} + +/** + * Check whether a request is authenticated by Authorization header or API key. + * Such requests are exempt from CSRF because browsers do not attach custom headers + * automatically in cross-site requests. + */ +function isHeaderAuthenticated(req) { + const authHeader = req.get('authorization'); + if (authHeader && authHeader.trim().length > 0) return true; + + const apiKeyHeader = req.get('x-api-key'); + if (apiKeyHeader && apiKeyHeader.trim().length > 0) return true; + + // Webhook signature header + const signatureHeader = req.get('x-webhook-signature') || req.get('x-signature'); + if (signatureHeader && signatureHeader.trim().length > 0) return true; + + return false; +} + +/** + * Check if the path matches the exemption list. + */ +function isExemptPath(path, customExemptions = []) { + if (!path) return false; + const cleanPath = path.split('?')[0]; + + const allExemptions = [...DEFAULT_EXEMPT_PATHS, ...customExemptions]; + return allExemptions.some((exempt) => { + if (exempt.endsWith('*')) { + return cleanPath.startsWith(exempt.slice(0, -1)); + } + return cleanPath === exempt || cleanPath === `${exempt}/`; + }); +} + +/** + * Create the global CSRF protection middleware. + */ +function createCsrfProtectionMiddleware(options = {}) { + const allowedOrigins = options.allowedOrigins || config.corsAllowedOrigins || []; + const exemptPaths = options.exemptPaths || []; + const secret = options.secret || CSRF_SECRET; + + return function csrfProtectionMiddleware(req, res, next) { + // 1. Detect and reject method override bypass attempts immediately + if (hasMethodOverride(req)) { + logger.warn('CSRF rejected: method-override attempt detected', { + path: req.originalUrl || req.url, + request_id: req.id, + }); + return next( + new AppError('FORBIDDEN', 'Method override is prohibited', 403) + ); + } + + // 2. Safe methods (GET, HEAD, OPTIONS) do not change state -> exempt + if (SAFE_METHODS.has(req.method.toUpperCase())) { + return next(); + } + + // 3. Explicitly declared route-level exemption flag + if (req.csrfExempt || req._csrfExempt) { + return next(); + } + + // 4. Path-based exemptions (health, metrics, docs, csrf-token endpoint) + const requestPath = req.path || req.originalUrl || ''; + if (isExemptPath(requestPath, exemptPaths)) { + return next(); + } + + // 5. Header-authenticated requests (Bearer token / API key / webhook signature) + // are exempt because browsers cannot attach custom Authorization headers cross-site. + if (isHeaderAuthenticated(req)) { + return next(); + } + + // 6. Origin / Referer validation for state-changing requests + if (allowedOrigins.length > 0 && !isAllowedOrigin(req, allowedOrigins)) { + logger.warn('CSRF rejected: Origin or Referer missing or not allowed', { + path: req.originalUrl || req.url, + method: req.method, + origin: req.get('origin') || '[missing]', + request_id: req.id, + }); + return next( + new AppError('FORBIDDEN', 'Cross-origin request rejected', 403) + ); + } + + // 7. CSRF token validation + // Token MUST be sent in a custom header; NEVER from query string. + const token = req.get('x-csrf-token') || req.get('x-xsrf-token'); + if (!token) { + logger.warn('CSRF rejected: Missing CSRF token header', { + path: req.originalUrl || req.url, + method: req.method, + request_id: req.id, + }); + return next( + new AppError('FORBIDDEN', 'Missing CSRF token', 403) + ); + } + + const sessionId = extractSessionId(req); + const isValid = verifyCsrfToken(token, sessionId, secret); + + if (!isValid) { + logger.warn('CSRF rejected: Invalid or mismatched CSRF token', { + path: req.originalUrl || req.url, + method: req.method, + request_id: req.id, + }); + return next( + new AppError('FORBIDDEN', 'Invalid CSRF token', 403) + ); + } + + return next(); + }; +} + +/** + * Express handler to obtain a new CSRF token bound to the current session. + */ +function csrfTokenHandler(req, res) { + const sessionId = extractSessionId(req); + const token = generateCsrfToken(sessionId); + res.json({ csrf_token: token }); +} + +/** + * Backward compatibility with webhook-specific CSRF header check (#320). */ function requireCsrfHeader(req, res, next) { - if (!req.get("x-requested-with")) { + if (!req.get('x-requested-with')) { return next( new AppError( - "CSRF_HEADER_REQUIRED", - "Missing required X-Requested-With header", - 403, - ), + 'CSRF_HEADER_REQUIRED', + 'Missing required X-Requested-With header', + 403 + ) ); } next(); } -module.exports = { requireCsrfHeader }; +module.exports = { + createCsrfProtectionMiddleware, + csrfProtectionMiddleware: createCsrfProtectionMiddleware(), + csrfTokenHandler, + generateCsrfToken, + verifyCsrfToken, + isAllowedOrigin, + hasMethodOverride, + isHeaderAuthenticated, + isExemptPath, + requireCsrfHeader, +}; diff --git a/test/csrfProtection.test.js b/test/csrfProtection.test.js new file mode 100644 index 0000000..c392be9 --- /dev/null +++ b/test/csrfProtection.test.js @@ -0,0 +1,281 @@ +'use strict'; + +const express = require('express'); +const request = require('supertest'); +const { + createCsrfProtectionMiddleware, + csrfTokenHandler, + generateCsrfToken, + verifyCsrfToken, + hasMethodOverride, +} = require('../src/middleware/csrf'); +const { errorHandler } = require('../src/middleware/errorHandler'); + +describe('Issue #465: Global CSRF Protection', () => { + const allowedOrigins = ['http://localhost:3000', 'https://smartdrop.io']; + const testSecret = 'test-csrf-secret-key-32-bytes!'; + + function createApp(options = {}) { + const app = express(); + app.use(express.json()); + + // Middleware to simulate session attachment from cookie if present + app.use((req, res, next) => { + const rawCookie = req.headers.cookie || ''; + const match = rawCookie.match(/(?:^|;\s*)session_id=([^;]+)/); + if (match) { + req.session = { id: decodeURIComponent(match[1]) }; + } + next(); + }); + + const csrf = createCsrfProtectionMiddleware({ + allowedOrigins: options.allowedOrigins || allowedOrigins, + exemptPaths: options.exemptPaths || ['/webhooks/signed-callback'], + secret: testSecret, + }); + + app.get('/csrf-token', csrfTokenHandler); + app.use(csrf); + + // Routes + app.get('/api/items', (req, res) => res.status(200).json({ items: [] })); + app.post('/api/items', (req, res) => res.status(201).json({ created: true })); + app.put('/api/items/:id', (req, res) => res.status(200).json({ updated: true })); + app.delete('/api/items/:id', (req, res) => res.status(200).json({ deleted: true })); + + // Declared webhook route (simulating webhook with signature or exempt path) + app.post('/webhooks/signed-callback', (req, res) => res.status(200).json({ webhook: true })); + + // Undeclared new route + app.post('/api/new-undeclared-feature', (req, res) => res.status(201).json({ success: true })); + + app.use(errorHandler); + return app; + } + + describe('1. State-Changing Route Protection', () => { + it('rejects a state-changing POST request without a CSRF token with 403 Forbidden', async () => { + const app = createApp(); + const res = await request(app) + .post('/api/items') + .set('Origin', 'http://localhost:3000') + .send({ name: 'Widget' }); + + expect(res.status).toBe(403); + expect(res.body.error).toBeDefined(); + expect(res.body.error.code).toBe('FORBIDDEN'); + expect(res.body.error.message).toContain('Missing CSRF token'); + }); + + it('allows a state-changing request with a valid token and allowed Origin', async () => { + const app = createApp(); + const sessionId = 'session-alice-123'; + const token = generateCsrfToken(sessionId, testSecret); + + const res = await request(app) + .post('/api/items') + .set('Cookie', `session_id=${sessionId}`) + .set('Origin', 'http://localhost:3000') + .set('X-CSRF-Token', token) + .send({ name: 'Widget' }); + + expect(res.status).toBe(201); + expect(res.body.created).toBe(true); + }); + + it('protects an undeclared new route by default', async () => { + const app = createApp(); + const res = await request(app) + .post('/api/new-undeclared-feature') + .set('Origin', 'http://localhost:3000') + .send({ action: 'doSomething' }); + + expect(res.status).toBe(403); + expect(res.body.error.code).toBe('FORBIDDEN'); + }); + }); + + describe('2. Session Binding & Tamper Resistance', () => { + it('rejects a token issued for another session', async () => { + const app = createApp(); + const bobSessionId = 'session-bob-456'; + const bobsToken = generateCsrfToken(bobSessionId, testSecret); + + // Alice tries to use Bob's token + const aliceSessionId = 'session-alice-123'; + const res = await request(app) + .post('/api/items') + .set('Cookie', `session_id=${aliceSessionId}`) + .set('Origin', 'http://localhost:3000') + .set('X-CSRF-Token', bobsToken) + .send({ name: 'Widget' }); + + expect(res.status).toBe(403); + expect(res.body.error.message).toContain('Invalid CSRF token'); + }); + + it('rejects a tampered or truncated token', async () => { + const app = createApp(); + const sessionId = 'session-alice-123'; + const validToken = generateCsrfToken(sessionId, testSecret); + const tamperedToken = validToken.slice(0, -6) + 'abcdef'; + + const res = await request(app) + .post('/api/items') + .set('Cookie', `session_id=${sessionId}`) + .set('Origin', 'http://localhost:3000') + .set('X-CSRF-Token', tamperedToken) + .send({ name: 'Widget' }); + + expect(res.status).toBe(403); + expect(res.body.error.message).toContain('Invalid CSRF token'); + }); + + it('never accepts CSRF token from the URL query string', async () => { + const app = createApp(); + const sessionId = 'session-alice-123'; + const token = generateCsrfToken(sessionId, testSecret); + + const res = await request(app) + .post(`/api/items?csrf_token=${token}&_csrf=${token}`) + .set('Cookie', `session_id=${sessionId}`) + .set('Origin', 'http://localhost:3000') + .send({ name: 'Widget' }); + + expect(res.status).toBe(403); + expect(res.body.error.message).toContain('Missing CSRF token'); + }); + }); + + describe('3. Safe Methods Exemption', () => { + it('exempts safe methods (GET, HEAD, OPTIONS) from requiring a token', async () => { + const app = createApp(); + const res = await request(app).get('/api/items'); + expect(res.status).toBe(200); + expect(res.body.items).toBeDefined(); + + const headRes = await request(app).head('/api/items'); + expect(headRes.status).toBe(200); + }); + }); + + describe('4. Origin and Referer Policy', () => { + it('rejects a state-changing request from a mismatched Origin even with a token', async () => { + const app = createApp(); + const sessionId = 'session-123'; + const token = generateCsrfToken(sessionId, testSecret); + + const res = await request(app) + .post('/api/items') + .set('Cookie', `session_id=${sessionId}`) + .set('Origin', 'http://attacker-site.evil.com') + .set('X-CSRF-Token', token) + .send({ name: 'Widget' }); + + expect(res.status).toBe(403); + expect(res.body.error.message).toContain('Cross-origin request rejected'); + }); + + it('accepts an allowed Origin', async () => { + const app = createApp(); + const sessionId = 'session-123'; + const token = generateCsrfToken(sessionId, testSecret); + + const res = await request(app) + .post('/api/items') + .set('Cookie', `session_id=${sessionId}`) + .set('Origin', 'https://smartdrop.io') + .set('X-CSRF-Token', token) + .send({ name: 'Widget' }); + + expect(res.status).toBe(201); + }); + + it('falls back to Referer when Origin is absent', async () => { + const app = createApp(); + const sessionId = 'session-123'; + const token = generateCsrfToken(sessionId, testSecret); + + const res = await request(app) + .post('/api/items') + .set('Cookie', `session_id=${sessionId}`) + .set('Referer', 'https://smartdrop.io/dashboard') + .set('X-CSRF-Token', token) + .send({ name: 'Widget' }); + + expect(res.status).toBe(201); + }); + }); + + describe('5. Header Authentication & Webhook Exemptions', () => { + it('exempts requests authenticated by Authorization header', async () => { + const app = createApp(); + const res = await request(app) + .post('/api/items') + .set('Authorization', 'Bearer valid-api-key') + .send({ name: 'API Widget' }); + + expect(res.status).toBe(201); + expect(res.body.created).toBe(true); + }); + + it('exempts declared webhook route verifying signatures', async () => { + const app = createApp(); + const res = await request(app) + .post('/webhooks/signed-callback') + .send({ event: 'delivery.success' }); + + expect(res.status).toBe(200); + expect(res.body.webhook).toBe(true); + }); + }); + + describe('6. Method Override Rejection', () => { + it('rejects X-HTTP-Method-Override header attempts to disguise requests', async () => { + const app = createApp(); + const res = await request(app) + .get('/api/items') + .set('X-HTTP-Method-Override', 'POST'); + + expect(res.status).toBe(403); + expect(res.body.error.message).toContain('Method override is prohibited'); + }); + + it('rejects ?_method=POST query parameter override', async () => { + const app = createApp(); + const res = await request(app).get('/api/items?_method=POST'); + + expect(res.status).toBe(403); + expect(res.body.error.message).toContain('Method override is prohibited'); + }); + }); + + describe('7. Token Endpoint and Constant-Time Comparison', () => { + it('provides a token endpoint returning the token in response body', async () => { + const app = createApp(); + const sessionId = 'user-sess-100'; + + const res = await request(app) + .get('/csrf-token') + .set('Cookie', `session_id=${sessionId}`); + + expect(res.status).toBe(200); + expect(res.body.csrf_token).toBeDefined(); + expect(typeof res.body.csrf_token).toBe('string'); + + // Validates successfully for that session + expect(verifyCsrfToken(res.body.csrf_token, sessionId, testSecret)).toBe(true); + }); + + it('performs timing-safe comparison on verifyCsrfToken', () => { + const sessionId = 'session-abc'; + const token = generateCsrfToken(sessionId, testSecret); + + expect(verifyCsrfToken(token, sessionId, testSecret)).toBe(true); + expect(verifyCsrfToken(token, 'different-session', testSecret)).toBe(false); + expect(verifyCsrfToken('', sessionId, testSecret)).toBe(false); + expect(verifyCsrfToken('invalid.format.extra', sessionId, testSecret)).toBe(false); + }); + }); +}); From a2398f76cf805925f00bbae51e5662e45bc798b8 Mon Sep 17 00:00:00 2001 From: zaytherr Date: Sun, 18 Oct 2026 03:43:15 +0100 Subject: [PATCH 4/4] feat(metrics): add Prometheus metrics endpoint (#467) --- .env.example | 14 +- README.md | 4 + src/metrics/index.js | 342 +++++++++++++++++++++++++++++++++++ src/metrics/registry.js | 251 +++++++++++++++++++++++++ src/routes/metrics.js | 144 +++++---------- test/metricsExporter.test.js | 216 ++++++++++++++++++++++ 6 files changed, 871 insertions(+), 100 deletions(-) create mode 100644 src/metrics/index.js create mode 100644 src/metrics/registry.js create mode 100644 test/metricsExporter.test.js diff --git a/.env.example b/.env.example index 03d3653..0041f95 100644 --- a/.env.example +++ b/.env.example @@ -167,7 +167,17 @@ SESSION_ABSOLUTE_LIFETIME_SECONDS=86400 SESSION_IDLE_TIMEOUT_SECONDS=1800 # SESSION_ACTIVITY_THROTTLE_SECONDS: minimum interval in seconds between store writes on activity touch. Default: 60. SESSION_ACTIVITY_THROTTLE_SECONDS=60 -# SESSION_CLOCK_SKEW_SECONDS: allowed clock-skew leeway in seconds for expiry comparisons. Default: 60. -SESSION_CLOCK_SKEW_SECONDS=60 +# CSRF protection (#465) +# CSRF_SECRET: secret key for signing CSRF tokens with HMAC-SHA256. +CSRF_SECRET= + +# Prometheus metrics exporter (#467) +# METRICS_ENABLED: boolean. Default: true. Returns 404 when false. +METRICS_ENABLED=true +# METRICS_AUTH_TOKEN: optional bearer token or password required to access GET /metrics. +METRICS_AUTH_TOKEN= +# METRICS_ALLOWED_IPS: optional comma-separated IP allowlist permitted to scrape /metrics. +METRICS_ALLOWED_IPS= + diff --git a/README.md b/README.md index 353a2a1..804c438 100644 --- a/README.md +++ b/README.md @@ -313,6 +313,10 @@ Each price source (`stellar_dex`, `coingecko`, `coinmarketcap`) gets one circuit | `SESSION_IDLE_TIMEOUT_SECONDS` | Inactivity timeout in seconds that activity extends | 1800 (30m) | No | | `SESSION_ACTIVITY_THROTTLE_SECONDS` | Minimum interval in seconds between session store activity writes | 60 | No | | `SESSION_CLOCK_SKEW_SECONDS` | Clock-skew leeway in seconds for expiry comparison | 60 | No | +| `CSRF_SECRET` | Secret key for signing per-session CSRF tokens (HMAC-SHA256) | auto-generated | No | +| `METRICS_ENABLED` | Enable Prometheus text exposition endpoint at `/metrics` | true | No | +| `METRICS_AUTH_TOKEN` | Bearer token / password for authenticating `/metrics` scrapes | empty | No | +| `METRICS_ALLOWED_IPS` | Comma-separated list of IP addresses allowed to scrape `/metrics` | empty | No | --- diff --git a/src/metrics/index.js b/src/metrics/index.js new file mode 100644 index 0000000..8f24a0b --- /dev/null +++ b/src/metrics/index.js @@ -0,0 +1,342 @@ +'use strict'; + +/** + * SmartDrop Prometheus Metrics Exporter (Issue #467). + * + * Implements: + * - Prometheus text exposition format (version 0.0.4) + * - HTTP metrics (requests_total, duration histogram, in_flight gauge) + * - Cardinality control: labeled by route template or 'unmatched', never raw URLs + * - Process metrics (CPU, memory, uptime, event loop lag) + * - Subsystem telemetry (cache hits/misses, Redis, circuit breakers, webhooks, indexer) + * - Endpoint protection (bearer token / basic credentials via constant-time comparison) + * - Avoids recursion: /metrics itself is excluded from HTTP traffic counts + */ + +const crypto = require('crypto'); +const { Registry } = require('./registry'); +const cache = require('../services/cache'); +const AppError = require('../errors/AppError'); +const logger = require('../logger'); + +const registry = new Registry(); + +// ── HTTP Metrics ───────────────────────────────────────────────────────────── +const httpRequestsTotal = registry.counter( + 'http_requests_total', + 'Total HTTP requests processed' +); + +const httpRequestDuration = registry.histogram( + 'http_request_duration_seconds', + 'HTTP request latency in seconds', + { buckets: [0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10] } +); + +const httpRequestsInFlight = registry.gauge( + 'http_requests_in_flight', + 'Current number of HTTP requests being processed' +); + +// ── Process Metrics ────────────────────────────────────────────────────────── +const processCpuUser = registry.counter( + 'process_cpu_user_seconds_total', + 'Total user CPU time spent in seconds' +); +const processCpuSystem = registry.counter( + 'process_cpu_system_seconds_total', + 'Total system CPU time spent in seconds' +); +const processResidentMemory = registry.gauge( + 'process_resident_memory_bytes', + 'Resident set size (RSS) memory in bytes' +); +const processHeapUsed = registry.gauge( + 'process_heap_bytes', + 'Process heap memory used in bytes' +); +const processHeapTotal = registry.gauge( + 'process_heap_total_bytes', + 'Total allocated process heap in bytes' +); +const processStartTime = registry.gauge( + 'process_start_time_seconds', + 'Start time of the process since unix epoch in seconds' +); +const processUptime = registry.gauge( + 'process_uptime_seconds', + 'Number of seconds the process has been running' +); +const eventLoopLagGauge = registry.gauge( + 'nodejs_eventloop_lag_seconds', + 'Event loop lag in seconds' +); + +// Set start time once +processStartTime.set({}, Math.floor(Date.now() / 1000 - process.uptime())); + +// ── Subsystem Metrics ──────────────────────────────────────────────────────── +const priceCacheHits = registry.counter( + 'price_cache_hits_total', + 'Total price cache hits' +); +const priceCacheMisses = registry.counter( + 'price_cache_misses_total', + 'Total price cache misses' +); +const webhookDeliveriesTotal = registry.counter( + 'webhook_deliveries_total', + 'Total webhook deliveries attempted' +); +const webhookDeliveryErrors = registry.counter( + 'webhook_delivery_errors_total', + 'Total failed webhook deliveries' +); +const redisConnectedGauge = registry.gauge( + 'redis_connected', + 'Whether Redis client is connected and ready (1 = connected, 0 = disconnected)' +); +const redisQueueDepthGauge = registry.gauge( + 'redis_command_queue_depth', + 'Number of pending commands in Redis client command queue' +); +const circuitBreakerStateGauge = registry.gauge( + 'circuit_breaker_state', + 'State of circuit breakers (0 = CLOSED, 1 = HALF_OPEN, 2 = OPEN)' +); + +// Event loop lag measurement +let lastLagCheck = Date.now(); +let eventLoopLagSeconds = 0; +const lagInterval = setInterval(() => { + const now = Date.now(); + const delta = now - lastLagCheck - 1000; + eventLoopLagSeconds = Math.max(0, delta) / 1000; + lastLagCheck = now; +}, 1000); +if (typeof lagInterval.unref === 'function') { + lagInterval.unref(); +} + +/** + * Safely resolves the matched route pattern without exposing high cardinality + * path parameters or leaking sensitive data. + */ +function resolveRoutePattern(req, res) { + // If request route is /metrics, return /metrics + if (req.path === '/metrics') return '/metrics'; + + // Express attaches route information when a router matches + if (req.route && req.route.path) { + const base = req.baseUrl || ''; + let routePath = req.route.path; + if (routePath instanceof RegExp) { + routePath = routePath.toString(); + } + return `${base}${routePath}`; + } + + // If status is 404 or unmatched + if (res.statusCode === 404) { + return 'unmatched'; + } + + // Handle standard known versioned paths + const cleanPath = (req.path || '').split('?')[0]; + if (cleanPath.startsWith('/health')) return '/health'; + if (cleanPath.startsWith('/api-docs')) return '/api-docs'; + if (cleanPath === '/csrf-token') return '/csrf-token'; + + return 'unmatched'; +} + +/** + * Express middleware to observe HTTP traffic. + * Placed early to count requests rejected by earlier middleware. + */ +function requestMetricsMiddleware(req, res, next) { + // Do NOT count /metrics itself to avoid recursive distortion of operational metrics + if (req.path === '/metrics') { + return next(); + } + + httpRequestsInFlight.inc(); + const startTime = process.hrtime.bigint(); + + res.on('finish', () => { + httpRequestsInFlight.dec(); + const durationSec = Number(process.hrtime.bigint() - startTime) / 1e9; + const method = req.method ? req.method.toUpperCase() : 'UNKNOWN'; + const status = String(res.statusCode || 200); + const route = resolveRoutePattern(req, res); + + const labels = { method, route, status }; + httpRequestsTotal.inc(labels); + httpRequestDuration.observe(labels, durationSec); + }); + + next(); +} + +/** + * Updates process telemetry right before serializing output. + */ +function updateProcessMetrics() { + const cpu = process.cpuUsage(); + processCpuUser.set({}, cpu.user / 1e6); + processCpuSystem.set({}, cpu.system / 1e6); + + const mem = process.memoryUsage(); + processResidentMemory.set({}, mem.rss); + processHeapUsed.set({}, mem.heapUsed); + processHeapTotal.set({}, mem.heapTotal); + + processUptime.set({}, Math.floor(process.uptime())); + eventLoopLagGauge.set({}, eventLoopLagSeconds); +} + +/** + * Updates subsystem telemetry right before serializing output. + */ +async function updateSubsystemMetrics() { + // Redis state + try { + const connected = cache.isConnected && cache.isConnected(); + redisConnectedGauge.set({}, connected ? 1 : 0); + const queueDepth = cache.getCommandQueueLength ? cache.getCommandQueueLength() : 0; + redisQueueDepthGauge.set({}, queueDepth); + } catch (err) { + redisConnectedGauge.set({}, 0); + } + + // Circuit breaker state + try { + const { getAllBreakerStates } = require('../utils/circuitBreaker'); + const breakerStates = getAllBreakerStates(); + for (const [name, info] of Object.entries(breakerStates)) { + const stateVal = info.state === 'OPEN' ? 2 : info.state === 'HALF_OPEN' ? 1 : 0; + circuitBreakerStateGauge.set({ name, state: info.state }, stateVal); + } + } catch { + // If circuitBreaker registry is absent or empty, omit + } +} + +/** + * Check metrics endpoint authorization. + */ +function checkMetricsAuth(req) { + const isEnabled = process.env.METRICS_ENABLED !== 'false'; + if (!isEnabled) { + return { authorized: false, reason: 'DISABLED' }; + } + + const expectedToken = process.env.METRICS_AUTH_TOKEN; + const isDev = (process.env.NODE_ENV || 'development') === 'development'; + + // If no auth token is configured and we are in development, allow access + if (!expectedToken) { + if (isDev) return { authorized: true }; + // Outside development, protected by default if credentials are not configured + return { authorized: false, reason: 'UNAUTHORIZED' }; + } + + // Check Bearer Token or Basic Auth + const authHeader = req.get('authorization') || ''; + let providedToken = null; + + if (authHeader.startsWith('Bearer ')) { + providedToken = authHeader.slice(7).trim(); + } else if (authHeader.startsWith('Basic ')) { + try { + const creds = Buffer.from(authHeader.slice(6), 'base64').toString('utf8'); + const parts = creds.split(':'); + providedToken = parts[1] || parts[0]; + } catch { + providedToken = null; + } + } + + if (!providedToken) { + return { authorized: false, reason: 'UNAUTHORIZED' }; + } + + const providedBuf = Buffer.from(providedToken, 'utf8'); + const expectedBuf = Buffer.from(expectedToken, 'utf8'); + + if (providedBuf.length !== expectedBuf.length || !crypto.timingSafeEqual(providedBuf, expectedBuf)) { + return { authorized: false, reason: 'UNAUTHORIZED' }; + } + + // Check IP allowlist if configured + const allowedIps = process.env.METRICS_ALLOWED_IPS + ? process.env.METRICS_ALLOWED_IPS.split(',').map((ip) => ip.trim()).filter(Boolean) + : []; + + if (allowedIps.length > 0) { + const clientIp = req.ip || req.socket?.remoteAddress || ''; + const isIpAllowed = allowedIps.some((allowed) => clientIp.includes(allowed)); + if (!isIpAllowed) { + return { authorized: false, reason: 'FORBIDDEN' }; + } + } + + return { authorized: true }; +} + +/** + * Express handler for GET /metrics in Prometheus text exposition format. + */ +async function metricsHandler(req, res) { + const authCheck = checkMetricsAuth(req); + if (!authCheck.authorized) { + if (authCheck.reason === 'DISABLED') { + return res.status(404).json({ + error: { code: 'NOT_FOUND', message: 'Metrics endpoint is disabled' }, + }); + } + if (authCheck.reason === 'FORBIDDEN') { + return res.status(403).json({ + error: { code: 'FORBIDDEN', message: 'Access forbidden from this IP' }, + }); + } + return res.status(401).json({ + error: { code: 'UNAUTHORIZED', message: 'Authentication required for metrics' }, + }); + } + + try { + updateProcessMetrics(); + await updateSubsystemMetrics(); + + const output = registry.serialize(); + res.setHeader('Content-Type', 'text/plain; version=0.0.4; charset=utf-8'); + return res.status(200).send(output); + } catch (err) { + logger.error('Failed to generate metrics', { error: err.message }); + return res.status(500).send('# Error generating metrics\n'); + } +} + +function incCounter(name, labels = {}) { + const m = registry.getMetric(name); + if (m && typeof m.inc === 'function') { + m.inc(labels); + } +} + +module.exports = { + registry, + metricsHandler, + requestMetricsMiddleware, + resolveRoutePattern, + checkMetricsAuth, + incCounter, + httpRequestsTotal, + httpRequestDuration, + httpRequestsInFlight, + priceCacheHits, + priceCacheMisses, + webhookDeliveriesTotal, + webhookDeliveryErrors, +}; diff --git a/src/metrics/registry.js b/src/metrics/registry.js new file mode 100644 index 0000000..c238c5b --- /dev/null +++ b/src/metrics/registry.js @@ -0,0 +1,251 @@ +'use strict'; + +/** + * Prometheus text exposition format generator and in-memory metrics registry (Issue #467). + * + * Implements: + * - Counter, Gauge, Histogram + * - HELP and TYPE lines + * - Label escaping + * - text/plain; version=0.0.4; charset=utf-8 output format + */ + +function escapeLabelValue(val) { + if (val === undefined || val === null) return ''; + return String(val) + .replace(/\\/g, '\\\\') + .replace(/"/g, '\\"') + .replace(/\n/g, '\\n'); +} + +function formatLabels(labels = {}) { + const entries = Object.entries(labels); + if (entries.length === 0) return ''; + const formatted = entries + .map(([k, v]) => `${k}="${escapeLabelValue(v)}"`) + .join(','); + return `{${formatted}}`; +} + +class Counter { + constructor(name, help) { + this.name = name; + this.help = help; + this.type = 'counter'; + this.values = new Map(); + } + + inc(labels = {}, value = 1) { + if (value < 0) return; + const key = JSON.stringify(labels); + const current = this.values.get(key) || { labels, value: 0 }; + current.value += value; + this.values.set(key, current); + } + + get(labels = {}) { + const key = JSON.stringify(labels); + return (this.values.get(key) || { value: 0 }).value; + } + + reset() { + this.values.clear(); + } + + serialize() { + const lines = []; + lines.push(`# HELP ${this.name} ${this.help}`); + lines.push(`# TYPE ${this.name} counter`); + if (this.values.size === 0) { + lines.push(`${this.name} 0`); + } else { + for (const entry of this.values.values()) { + lines.push(`${this.name}${formatLabels(entry.labels)} ${entry.value}`); + } + } + return lines.join('\n'); + } +} + +class Gauge { + constructor(name, help) { + this.name = name; + this.help = help; + this.type = 'gauge'; + this.values = new Map(); + } + + set(labels = {}, value) { + const key = JSON.stringify(labels); + this.values.set(key, { labels, value: Number(value) }); + } + + inc(labels = {}, value = 1) { + const key = JSON.stringify(labels); + const current = this.values.get(key) || { labels, value: 0 }; + current.value += value; + this.values.set(key, current); + } + + dec(labels = {}, value = 1) { + this.inc(labels, -value); + } + + get(labels = {}) { + const key = JSON.stringify(labels); + return (this.values.get(key) || { value: 0 }).value; + } + + reset() { + this.values.clear(); + } + + serialize() { + const lines = []; + lines.push(`# HELP ${this.name} ${this.help}`); + lines.push(`# TYPE ${this.name} gauge`); + if (this.values.size === 0) { + lines.push(`${this.name} 0`); + } else { + for (const entry of this.values.values()) { + lines.push(`${this.name}${formatLabels(entry.labels)} ${entry.value}`); + } + } + return lines.join('\n'); + } +} + +const DEFAULT_BUCKETS = [ + 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, +]; + +class Histogram { + constructor(name, help, options = {}) { + this.name = name; + this.help = help; + this.type = 'histogram'; + this.buckets = options.buckets || DEFAULT_BUCKETS; + this.entries = new Map(); + } + + observe(labels = {}, value) { + const val = Number(value); + const key = JSON.stringify(labels); + let entry = this.entries.get(key); + if (!entry) { + const bucketCounts = {}; + this.buckets.forEach((b) => { + bucketCounts[b] = 0; + }); + bucketCounts['+Inf'] = 0; + entry = { + labels, + sum: 0, + count: 0, + bucketCounts, + }; + this.entries.set(key, entry); + } + + entry.sum += val; + entry.count += 1; + + for (const b of this.buckets) { + if (val <= b) { + entry.bucketCounts[b] += 1; + } + } + entry.bucketCounts['+Inf'] += 1; + } + + reset() { + this.entries.clear(); + } + + serialize() { + const lines = []; + lines.push(`# HELP ${this.name} ${this.help}`); + lines.push(`# TYPE ${this.name} histogram`); + + if (this.entries.size === 0) { + lines.push(`${this.name}_count 0`); + lines.push(`${this.name}_sum 0`); + } else { + for (const entry of this.entries.values()) { + const baseLabels = entry.labels; + for (const b of this.buckets) { + const bLabels = { ...baseLabels, le: String(b) }; + lines.push( + `${this.name}_bucket${formatLabels(bLabels)} ${entry.bucketCounts[b]}` + ); + } + lines.push( + `${this.name}_bucket${formatLabels({ ...baseLabels, le: '+Inf' })} ${entry.bucketCounts['+Inf']}` + ); + lines.push( + `${this.name}_sum${formatLabels(baseLabels)} ${Number(entry.sum.toFixed(6))}` + ); + lines.push( + `${this.name}_count${formatLabels(baseLabels)} ${entry.count}` + ); + } + } + return lines.join('\n'); + } +} + +class Registry { + constructor() { + this.metrics = new Map(); + } + + register(metric) { + this.metrics.set(metric.name, metric); + return metric; + } + + counter(name, help) { + if (this.metrics.has(name)) return this.metrics.get(name); + const metric = new Counter(name, help); + return this.register(metric); + } + + gauge(name, help) { + if (this.metrics.has(name)) return this.metrics.get(name); + const metric = new Gauge(name, help); + return this.register(metric); + } + + histogram(name, help, options) { + if (this.metrics.has(name)) return this.metrics.get(name); + const metric = new Histogram(name, help, options); + return this.register(metric); + } + + getMetric(name) { + return this.metrics.get(name); + } + + reset() { + for (const metric of this.metrics.values()) { + metric.reset(); + } + } + + serialize() { + const blocks = []; + for (const metric of this.metrics.values()) { + blocks.push(metric.serialize()); + } + return blocks.join('\n') + '\n'; + } +} + +module.exports = { + Registry, + Counter, + Gauge, + Histogram, + escapeLabelValue, + formatLabels, +}; diff --git a/src/routes/metrics.js b/src/routes/metrics.js index 883ec0e..6570715 100644 --- a/src/routes/metrics.js +++ b/src/routes/metrics.js @@ -1,108 +1,56 @@ 'use strict'; +/** + * Metrics Router (#467). + * Connects Express router with the Prometheus text exporter in src/metrics/. + */ + const express = require('express'); -const cache = require('../services/cache'); -const eventStore = require('../indexer/eventStore'); -const indexerPoller = require('../indexer/runtime'); -const webhookRepo = require('../repositories/webhookRepository'); -const deliveryRepo = require('../repositories/deliveryRepository'); +const { + metricsHandler, + requestMetricsMiddleware, + incCounter, + registry, + httpRequestsTotal, +} = require('../metrics'); const router = express.Router(); -// Simple in-memory counters (reset on process restart) -const counters = { - http_requests_total: 0, - http_errors_total: 0, - webhook_deliveries_total: 0, - webhook_delivery_errors_total: 0, - price_cache_hits: 0, - price_cache_misses: 0, -}; - -// Track request metrics via a middleware -function requestMetricsMiddleware(req, res, next) { - counters.http_requests_total++; - const originalEnd = res.end; - res.end = function (...args) { - if (res.statusCode >= 400) { - counters.http_errors_total++; - } - originalEnd.apply(this, args); - }; - next(); -} - -function incCounter(name) { - counters[name] = (counters[name] || 0) + 1; -} - -async function metricsHandler(_req, res) { - try { - const lines = []; - - // HTTP metrics - lines.push('# HELP http_requests_total Total HTTP requests'); - lines.push('# TYPE http_requests_total counter'); - lines.push(`http_requests_total ${counters.http_requests_total}`); - - lines.push('# HELP http_errors_total Total HTTP errors (4xx+5xx)'); - lines.push('# TYPE http_errors_total counter'); - lines.push(`http_errors_total ${counters.http_errors_total}`); - - // Webhook metrics - lines.push('# HELP webhook_deliveries_total Total webhook deliveries attempted'); - lines.push('# TYPE webhook_deliveries_total counter'); - lines.push(`webhook_deliveries_total ${counters.webhook_deliveries_total}`); - - lines.push('# HELP webhook_delivery_errors_total Total failed webhook deliveries'); - lines.push('# TYPE webhook_delivery_errors_total counter'); - lines.push(`webhook_delivery_errors_total ${counters.webhook_delivery_errors_total}`); - - // Price cache metrics - lines.push('# HELP price_cache_hits_total Price cache hits'); - lines.push('# TYPE price_cache_hits_total counter'); - lines.push(`price_cache_hits_total ${counters.price_cache_hits}`); - - lines.push('# HELP price_cache_misses_total Price cache misses'); - lines.push('# TYPE price_cache_misses_total counter'); - lines.push(`price_cache_misses_total ${counters.price_cache_misses}`); - - // Indexer status - const indexerStatus = indexerPoller.getStatus(); - lines.push('# HELP indexer_running Whether the indexer poller is running'); - lines.push('# TYPE indexer_running gauge'); - lines.push(`indexer_running ${indexerStatus.running ? 1 : 0}`); - - lines.push('# HELP indexer_latest_ledger Latest ledger processed by indexer'); - lines.push('# TYPE indexer_latest_ledger gauge'); - lines.push(`indexer_latest_ledger ${indexerStatus.latest_ledger || 0}`); - - // Indexer event count - const stats = await eventStore.getStats(); - lines.push('# HELP indexer_events_total Total indexed events'); - lines.push('# TYPE indexer_events_total gauge'); - lines.push(`indexer_events_total ${stats.events_count}`); - - // Active webhooks - const allWebhooks = await webhookRepo.listAll(); - const activeWebhooks = allWebhooks.filter((w) => w.active); - lines.push('# HELP webhooks_active Number of active webhooks'); - lines.push('# TYPE webhooks_active gauge'); - lines.push(`webhooks_active ${activeWebhooks.length}`); - - // Redis connectivity - const redisConnected = cache.isConnected(); - lines.push('# HELP redis_connected Whether Redis is connected'); - lines.push('# TYPE redis_connected gauge'); - lines.push(`redis_connected ${redisConnected ? 1 : 0}`); - - res.setHeader('Content-Type', 'text/plain; version=0.0.4; charset=utf-8'); - res.send(lines.join('\n') + '\n'); - } catch (err) { - res.status(500).send('# Error generating metrics\n'); +// Forward compatibility + backward compatibility proxy for legacy counters object +const counters = new Proxy( + {}, + { + get(_target, prop) { + if (prop === 'http_requests_total') { + let sum = 0; + for (const entry of httpRequestsTotal.values.values()) { + sum += entry.value; + } + return sum; + } + const metric = registry.getMetric(prop); + if (metric && typeof metric.get === 'function') { + return metric.get(); + } + return 0; + }, + set(_target, prop, val) { + const metric = registry.getMetric(prop); + if (metric && typeof metric.inc === 'function') { + metric.inc({}, val); + } + return true; + }, } -} +); router.get('/metrics', metricsHandler); -module.exports = { router, requestMetricsMiddleware, incCounter, counters }; +module.exports = { + router, + metricsHandler, + requestMetricsMiddleware, + incCounter, + counters, + registry, +}; diff --git a/test/metricsExporter.test.js b/test/metricsExporter.test.js new file mode 100644 index 0000000..2123078 --- /dev/null +++ b/test/metricsExporter.test.js @@ -0,0 +1,216 @@ +'use strict'; + +const express = require('express'); +const request = require('supertest'); +const { + Registry, + escapeLabelValue, + formatLabels, +} = require('../src/metrics/registry'); +const { + registry, + metricsHandler, + requestMetricsMiddleware, + httpRequestsTotal, + httpRequestDuration, +} = require('../src/metrics'); +const { errorHandler } = require('../src/middleware/errorHandler'); + +describe('Issue #467: Prometheus Metrics Exporter', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + registry.reset(); + }); + + afterAll(() => { + process.env = originalEnv; + }); + + describe('1. Exposition Format & Content-Type', () => { + it('returns text/plain; version=0.0.4; charset=utf-8 with valid HELP and TYPE lines', async () => { + process.env.METRICS_ENABLED = 'true'; + delete process.env.METRICS_AUTH_TOKEN; + + const app = express(); + app.get('/metrics', metricsHandler); + + const res = await request(app).get('/metrics'); + + expect(res.status).toBe(200); + expect(res.headers['content-type']).toBe('text/plain; version=0.0.4; charset=utf-8'); + expect(res.text).toContain('# HELP http_requests_total'); + expect(res.text).toContain('# TYPE http_requests_total counter'); + expect(res.text).toContain('# HELP http_request_duration_seconds'); + expect(res.text).toContain('# TYPE http_request_duration_seconds histogram'); + }); + + it('correctly escapes label values with quotes, backslashes, and newlines', () => { + const reg = new Registry(); + const counter = reg.counter('test_escape_total', 'Test escaping'); + + counter.inc({ reason: 'quotes " and \\ and \n newline' }); + const serialized = reg.serialize(); + + expect(serialized).toContain('reason="quotes \\" and \\\\ and \\n newline"'); + }); + }); + + describe('2. HTTP Metrics Tracking & Route Template Cardinality', () => { + function createTestApp() { + const app = express(); + app.use(requestMetricsMiddleware); + + const router = express.Router(); + router.get('/airdrops/:id', (req, res) => res.json({ id: req.params.id })); + router.post('/airdrops', (req, res) => res.status(201).json({ created: true })); + + app.use('/v1', router); + app.get('/metrics', metricsHandler); + + app.use((req, res) => res.status(404).json({ error: 'not found' })); + return app; + } + + it('increments counters and histograms for handled requests using the route template', async () => { + const app = createTestApp(); + + // Request a parameterized route with user ID + const res = await request(app).get('/v1/airdrops/airdrop-uuid-12345'); + expect(res.status).toBe(200); + + const metricsRes = await request(app).get('/metrics'); + expect(metricsRes.text).toContain( + 'http_requests_total{method="GET",route="/v1/airdrops/:id",status="200"} 1' + ); + expect(metricsRes.text).toContain( + 'http_request_duration_seconds_count{method="GET",route="/v1/airdrops/:id",status="200"} 1' + ); + + // Ensures the specific ID was not incorporated into the route label (bounded cardinality) + expect(metricsRes.text).not.toContain('airdrop-uuid-12345'); + }); + + it('labels unmatched routes with the fixed label "unmatched"', async () => { + const app = createTestApp(); + + const res = await request(app).get('/v1/non-existent-random-path'); + expect(res.status).toBe(404); + + const metricsRes = await request(app).get('/metrics'); + expect(metricsRes.text).toContain( + 'http_requests_total{method="GET",route="unmatched",status="404"} 1' + ); + }); + + it('does NOT count /metrics requests to avoid recursion and scraping distortion', async () => { + const app = createTestApp(); + + await request(app).get('/metrics'); + await request(app).get('/metrics'); + + const metricsRes = await request(app).get('/metrics'); + expect(metricsRes.text).not.toContain('route="/metrics"'); + }); + }); + + describe('3. Rejection by Earlier Middleware Counting', () => { + it('counts requests that are rejected by earlier middleware (e.g. rate-limit or auth)', async () => { + const app = express(); + // Metrics middleware sits first + app.use(requestMetricsMiddleware); + + // Simulating rejecting middleware (e.g. 429 rate limit or 403 forbidden) + app.use('/api/rejected', (req, res) => { + return res.status(429).json({ error: 'Rate limit exceeded' }); + }); + + app.get('/metrics', metricsHandler); + + const res = await request(app).get('/api/rejected'); + expect(res.status).toBe(429); + + const metricsRes = await request(app).get('/metrics'); + expect(metricsRes.text).toContain( + 'http_requests_total{method="GET",route="unmatched",status="429"} 1' + ); + }); + }); + + describe('4. Protection and Authentication', () => { + it('returns 404 when metrics are disabled via METRICS_ENABLED=false', async () => { + process.env.METRICS_ENABLED = 'false'; + + const app = express(); + app.get('/metrics', metricsHandler); + + const res = await request(app).get('/metrics'); + expect(res.status).toBe(404); + expect(res.body.error.code).toBe('NOT_FOUND'); + }); + + it('rejects missing or incorrect credentials when METRICS_AUTH_TOKEN is configured', async () => { + process.env.METRICS_ENABLED = 'true'; + process.env.METRICS_AUTH_TOKEN = 'secret-metrics-token-123'; + + const app = express(); + app.get('/metrics', metricsHandler); + + // Missing token + const noAuthRes = await request(app).get('/metrics'); + expect(noAuthRes.status).toBe(401); + + // Wrong token + const wrongAuthRes = await request(app) + .get('/metrics') + .set('Authorization', 'Bearer wrong-token'); + expect(wrongAuthRes.status).toBe(401); + + // Valid token + const validAuthRes = await request(app) + .get('/metrics') + .set('Authorization', 'Bearer secret-metrics-token-123'); + expect(validAuthRes.status).toBe(200); + expect(validAuthRes.headers['content-type']).toBe('text/plain; version=0.0.4; charset=utf-8'); + }); + + it('denies requests from unauthorized IPs when METRICS_ALLOWED_IPS is configured', async () => { + process.env.METRICS_ENABLED = 'true'; + process.env.METRICS_AUTH_TOKEN = 'token'; + process.env.METRICS_ALLOWED_IPS = '10.0.0.1,10.0.0.2'; + + const app = express(); + app.get('/metrics', metricsHandler); + + const res = await request(app) + .get('/metrics') + .set('Authorization', 'Bearer token'); + + // Supertest default remote address is 127.0.0.1, not in 10.0.0.1 + expect(res.status).toBe(403); + expect(res.body.error.code).toBe('FORBIDDEN'); + }); + }); + + describe('5. Data Privacy & Leaks', () => { + it('contains no secret tokens, passwords, or personal email addresses in metric output', async () => { + process.env.METRICS_ENABLED = 'true'; + delete process.env.METRICS_AUTH_TOKEN; + + const app = express(); + app.use(requestMetricsMiddleware); + app.get('/v1/users/:id', (req, res) => res.json({ id: req.params.id })); + app.get('/metrics', metricsHandler); + + await request(app) + .get('/v1/users/alice@example.com?secret_token=topsecret123') + .set('Authorization', 'Bearer my-private-token'); + + const metricsRes = await request(app).get('/metrics'); + expect(metricsRes.text).not.toContain('alice@example.com'); + expect(metricsRes.text).not.toContain('topsecret123'); + expect(metricsRes.text).not.toContain('my-private-token'); + }); + }); +});