From b0cbc30f146f5e2d98c6622aa7c8c1fb5ef90d0b Mon Sep 17 00:00:00 2001 From: fretuks Date: Tue, 30 Jun 2026 09:46:23 +0200 Subject: [PATCH] add overall more stability --- README.md | 7 +- scripts/lint.mjs | 1 + src/app.js | 147 +++++++++++++++++++++++------------- src/config.js | 31 +++++++- src/middleware/rateLimit.js | 33 +++++++- test/app.test.js | 1 + 6 files changed, 161 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index 519793a..d539882 100644 --- a/README.md +++ b/README.md @@ -20,8 +20,12 @@ KrisHD_AI is an Express + SQLite app for account-based chat, persona management, - `PORT`: HTTP port for the Express server. - `DB_PATH`: SQLite database path. Use `:memory:` for isolated tests. - `SESSION_SECRET`: session signing secret. Required outside local throwaway environments. -- `COOKIE_SECURE`: set to `true` when serving over HTTPS. +- `COOKIE_SECURE`: set to `true` when serving over HTTPS. Required in production unless `ALLOW_INSECURE_COOKIES=true` is set. +- `ALLOW_INSECURE_COOKIES`: explicit production override for non-HTTPS deployments. +- `TRUST_PROXY`: Express trust proxy setting for reverse proxy deployments. - `SESSION_MAX_AGE_MS`: cookie lifetime. +- `EVENT_LOOP_LAG_*`: enable and tune event-loop lag warnings. +- `SLOW_REQUEST_*`: enable and tune slow request warnings. - `CHAT_LIMIT`: per-account maximum chat sessions. - `CHAT_HISTORY_LIMIT`: number of recent messages included in model prompts. - `MODEL_API_BASE_URL`: base URL of the remote model backend. The server expects `/chat` and `/tags`. @@ -31,6 +35,7 @@ KrisHD_AI is an Express + SQLite app for account-based chat, persona management, - `MODELS_CACHE_TTL_MS`: model list cache TTL. - `AUTH_RATE_LIMIT_*`: rate-limit window and thresholds for registration/login. - `CHAT_RATE_LIMIT_*`: rate-limit window and thresholds for chat and retry endpoints. +- `RATE_LIMIT_CLEANUP_INTERVAL_MS`: interval for pruning stale in-memory rate-limit buckets. - `DROP_LEGACY_CHATS`: when `true`, migrate any legacy `chats` table into `chat_sessions` / `chat_messages` and remove the old table. ## Model Backend Expectations diff --git a/scripts/lint.mjs b/scripts/lint.mjs index 379ee32..fcb2e28 100644 --- a/scripts/lint.mjs +++ b/scripts/lint.mjs @@ -9,6 +9,7 @@ const files = [ "src/db/schema.js", "src/middleware/auth.js", "src/middleware/rateLimit.js", + "src/middleware/rateLimiters.js", "src/middleware/validate.js", "src/routes/auth.js", "src/routes/chats.js", diff --git a/src/app.js b/src/app.js index 5b9ea8a..9a1af48 100644 --- a/src/app.js +++ b/src/app.js @@ -6,7 +6,7 @@ import FileStoreFactory from "session-file-store"; import {createConfig} from "./config.js"; import {createDatabase} from "./db/index.js"; import {createRepositories} from "./db/repositories.js"; -import {createRateLimiter} from "./middleware/rateLimit.js"; +import {createAuthRateLimiters, createChatRateLimiters} from "./middleware/rateLimiters.js"; import {createModelService} from "./services/modelService.js"; import {createChatService} from "./services/chatService.js"; import {createAuthRouter} from "./routes/auth.js"; @@ -16,72 +16,104 @@ import {createPersonasRouter} from "./routes/personas.js"; import {createSettingsRouter} from "./routes/settings.js"; import {createSystemRouter} from "./routes/system.js"; +function createCloseHandler(resources) { + let closed = false; + return async () => { + if (closed) return; + closed = true; + + for (const close of resources) { + await close(); + } + }; +} + +function createLifecycleMonitors(app, config) { + const resources = []; + + if (config.eventLoopLagMonitorEnabled) { + let last = performance.now(); + const lagMonitor = setInterval(() => { + const now = performance.now(); + const drift = now - last - 1000; + last = now; + if (drift > config.eventLoopLagWarnMs) { + console.warn("EVENT LOOP LAG", Math.round(drift), "ms", new Date().toISOString()); + } + }, 1000); + lagMonitor.unref(); + resources.push(() => clearInterval(lagMonitor)); + } + + if (config.slowRequestLoggingEnabled) { + app.use((req, res, next) => { + const start = Date.now(); + res.on("finish", () => { + const ms = Date.now() - start; + if (ms > config.slowRequestWarnMs) { + console.warn("SLOW REQ", `${ms}ms`, req.method, req.url); + } + }); + next(); + }); + } + + return resources; +} + +function wantsJson(req) { + const accept = req.get("Accept") || ""; + const contentType = req.get("Content-Type") || ""; + return req.xhr + || req.path.startsWith("/api/") + || accept.includes("application/json") + || contentType.includes("application/json"); +} + +function notFoundHandler(req, res) { + if (wantsJson(req)) { + return res.status(404).json({error: "Not found"}); + } + return res.status(404).type("text").send("Not found"); +} + +function errorHandler(err, req, res, next) { + if (res.headersSent) { + return next(err); + } + + const status = Number.isInteger(err.status) ? err.status : 500; + const message = status >= 500 ? "Internal server error" : err.message; + console.warn("REQUEST ERROR", status, req.method, req.url, err); + if (wantsJson(req)) { + return res.status(status).json({error: message}); + } + return res.status(status).type("text").send(message); +} + export function createApp(options = {}) { const config = createConfig(options.config); const app = express(); + app.set("trust proxy", config.trustProxy); if (config.dbPath !== ":memory:") { fs.mkdirSync(config.sessionsDir, {recursive: true}); } - let last = performance.now(); - const lagMonitor = setInterval(() => { - const now = performance.now(); - const drift = now - last - 1000; - last = now; - if (drift > 50) { - console.log("EVENT LOOP LAG", Math.round(drift), "ms", new Date().toISOString()); - } - }, 1000); - lagMonitor.unref(); - - app.use((req, res, next) => { - const start = Date.now(); - res.on("finish", () => { - const ms = Date.now() - start; - if (ms > 50) console.log("SLOW REQ", `${ms}ms`, req.method, req.url); - }); - next(); - }); + const closeHandlers = createLifecycleMonitors(app, config); const db = createDatabase(config); const repositories = createRepositories(db, config); const modelService = options.modelService || createModelService(config, options.modelOverrides); const chatService = createChatService(repositories, modelService, config); const FileStore = FileStoreFactory(session); - - const authRateLimiters = [ - createRateLimiter({ - windowMs: config.authRateLimitWindowMs, - max: config.authRateLimitMaxPerIp, - keyGenerator: (req) => `auth:ip:${req.ip}`, - message: "Too many authentication attempts from this IP. Please try again later." - }), - createRateLimiter({ - windowMs: config.authRateLimitWindowMs, - max: config.authRateLimitMaxPerAccount, - keyGenerator: (req) => { - const username = String(req.body?.username || "").trim().toLowerCase(); - return username ? `auth:user:${username}` : null; - }, - message: "Too many authentication attempts for this account. Please try again later." - }) - ]; - - const chatRateLimiters = [ - createRateLimiter({ - windowMs: config.chatRateLimitWindowMs, - max: config.chatRateLimitMaxPerIp, - keyGenerator: (req) => `chat:ip:${req.ip}`, - message: "Too many chat requests from this IP. Please slow down." - }), - createRateLimiter({ - windowMs: config.chatRateLimitWindowMs, - max: config.chatRateLimitMaxPerAccount, - keyGenerator: (req) => req.session.user ? `chat:user:${req.session.user}` : null, - message: "Too many chat requests for this account. Please slow down." - }) - ]; + const authRateLimiters = createAuthRateLimiters(config); + const chatRateLimiters = createChatRateLimiters(config); + closeHandlers.push( + ...authRateLimiters.map((limiter) => limiter.close), + ...chatRateLimiters.map((limiter) => limiter.close), + () => db.close() + ); app.use(express.json({limit: "1mb"})); app.use(express.static(config.publicDir)); @@ -102,6 +134,7 @@ export function createApp(options = {}) { app.locals.db = db; app.locals.repositories = repositories; app.locals.modelService = modelService; + app.locals.close = createCloseHandler(closeHandlers); app.use(createAuthRouter({repositories, authRateLimiters})); app.use(createSettingsRouter({repositories})); @@ -109,6 +142,8 @@ export function createApp(options = {}) { app.use(createPersonasRouter({repositories, chatService, modelService, config})); app.use(createSystemRouter({modelService})); app.use(createPagesRouter(config)); + app.use(notFoundHandler); + app.use(errorHandler); return app; } @@ -119,5 +154,9 @@ export function startServer(options = {}) { const server = app.listen(port, () => { console.log(`Server running at http://localhost:${port}`); }); - return {app, server}; + const close = async () => { + await new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())); + await app.locals.close(); + }; + return {app, server, close}; } diff --git a/src/config.js b/src/config.js index f0f7c28..abc8713 100644 --- a/src/config.js +++ b/src/config.js @@ -10,10 +10,20 @@ const int = (value, fallback) => { return Number.isInteger(parsed) ? parsed : fallback; }; +const trustProxy = (value, fallback = false) => { + if (value == null || value === "") return fallback; + const normalized = String(value).toLowerCase(); + if (["1", "true", "yes", "on"].includes(normalized)) return true; + if (["0", "false", "no", "off"].includes(normalized)) return false; + const parsed = Number.parseInt(value, 10); + return Number.isInteger(parsed) ? parsed : value; +}; + export function createConfig(overrides = {}) { const rootDir = overrides.rootDir || process.cwd(); const dataDir = overrides.dataDir || path.resolve(rootDir, "data"); const sessionsDir = overrides.sessionsDir || path.resolve(rootDir, "sessions"); + const testMode = bool(process.env.TEST_MODE, process.env.NODE_ENV === "test"); const config = { rootDir, @@ -24,7 +34,13 @@ export function createConfig(overrides = {}) { dbPath: process.env.DB_PATH || path.resolve(dataDir, "app.db"), sessionSecret: process.env.SESSION_SECRET || "change-me-session-secret", cookieSecure: bool(process.env.COOKIE_SECURE, false), + allowInsecureCookies: bool(process.env.ALLOW_INSECURE_COOKIES, false), + trustProxy: trustProxy(process.env.TRUST_PROXY, false), sessionMaxAgeMs: int(process.env.SESSION_MAX_AGE_MS, 7 * 24 * 60 * 60 * 1000), + eventLoopLagMonitorEnabled: bool(process.env.EVENT_LOOP_LAG_MONITOR_ENABLED, !testMode), + eventLoopLagWarnMs: int(process.env.EVENT_LOOP_LAG_WARN_MS, 50), + slowRequestLoggingEnabled: bool(process.env.SLOW_REQUEST_LOGGING_ENABLED, !testMode), + slowRequestWarnMs: int(process.env.SLOW_REQUEST_WARN_MS, 50), chatLimit: int(process.env.CHAT_LIMIT, 10), chatHistoryLimit: int(process.env.CHAT_HISTORY_LIMIT, 20), modelApiBaseUrl: process.env.MODEL_API_BASE_URL || "https://ai.krishd.ch/api", @@ -38,9 +54,20 @@ export function createConfig(overrides = {}) { chatRateLimitWindowMs: int(process.env.CHAT_RATE_LIMIT_WINDOW_MS, 60 * 1000), chatRateLimitMaxPerIp: int(process.env.CHAT_RATE_LIMIT_MAX_PER_IP, 60), chatRateLimitMaxPerAccount: int(process.env.CHAT_RATE_LIMIT_MAX_PER_ACCOUNT, 30), - testMode: bool(process.env.TEST_MODE, process.env.NODE_ENV === "test"), + rateLimitCleanupIntervalMs: int(process.env.RATE_LIMIT_CLEANUP_INTERVAL_MS, 60 * 1000), + testMode, dropLegacyChats: bool(process.env.DROP_LEGACY_CHATS, true) }; - return {...config, ...overrides}; + const resolved = {...config, ...overrides}; + if (process.env.NODE_ENV === "production" && !resolved.testMode) { + if (resolved.sessionSecret === "change-me-session-secret") { + throw new Error("SESSION_SECRET must be set in production."); + } + if (!resolved.cookieSecure && !resolved.allowInsecureCookies) { + throw new Error("COOKIE_SECURE must be true in production unless ALLOW_INSECURE_COOKIES=true is set."); + } + } + + return resolved; } diff --git a/src/middleware/rateLimit.js b/src/middleware/rateLimit.js index 8eb79c7..cf915f1 100644 --- a/src/middleware/rateLimit.js +++ b/src/middleware/rateLimit.js @@ -5,10 +5,26 @@ function pruneBucket(bucket, windowMs, now) { } } -export function createRateLimiter({windowMs, max, keyGenerator, message}) { +export function createRateLimiter({windowMs, max, keyGenerator, message, cleanupIntervalMs = 60 * 1000}) { const hits = new Map(); + let cleanupTimer = null; - return (req, res, next) => { + const cleanup = () => { + const now = Date.now(); + for (const [key, bucket] of hits) { + pruneBucket(bucket, windowMs, now); + if (!bucket.length) { + hits.delete(key); + } + } + }; + + if (cleanupIntervalMs > 0) { + cleanupTimer = setInterval(cleanup, cleanupIntervalMs); + cleanupTimer.unref(); + } + + const limiter = (req, res, next) => { const key = keyGenerator(req); if (!key) { return next(); @@ -17,6 +33,9 @@ export function createRateLimiter({windowMs, max, keyGenerator, message}) { const now = Date.now(); const bucket = hits.get(key) || []; pruneBucket(bucket, windowMs, now); + if (!bucket.length) { + hits.delete(key); + } if (bucket.length >= max) { return res.status(429).json({error: message}); } @@ -25,4 +44,14 @@ export function createRateLimiter({windowMs, max, keyGenerator, message}) { hits.set(key, bucket); return next(); }; + + limiter.close = () => { + if (cleanupTimer) { + clearInterval(cleanupTimer); + cleanupTimer = null; + } + hits.clear(); + }; + + return limiter; } diff --git a/test/app.test.js b/test/app.test.js index 6bf25bf..ade8046 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -69,6 +69,7 @@ async function startTestServer(overrides = {}) { baseUrl, async close() { await new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())); + await app.locals.close(); } }; }