add overall more stability

2026-06-30 09:46:23 +02:00
parent 58df321d8d
commit b0cbc30f14
6 changed files with 161 additions and 59 deletions
@@ -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
@@ -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",
@@ -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};
 }
@@ -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;
 }
@@ -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;
 }
@@ -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();
        }
    };
 }