From ae0da2d620b91371bc753a7de81aa8a94220a38c Mon Sep 17 00:00:00 2001 From: fretuks Date: Tue, 30 Jun 2026 11:31:41 +0200 Subject: [PATCH] Implement overall stability fixes including session store injection --- src/app.js | 99 +++++++++++++++++++++++++++++--- src/config.js | 4 ++ test/app.test.js | 143 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 236 insertions(+), 10 deletions(-) diff --git a/src/app.js b/src/app.js index 9a1af48..2d80862 100644 --- a/src/app.js +++ b/src/app.js @@ -16,14 +16,23 @@ import {createPersonasRouter} from "./routes/personas.js"; import {createSettingsRouter} from "./routes/settings.js"; import {createSystemRouter} from "./routes/system.js"; -function createCloseHandler(resources) { +export function createCloseHandler(resources) { let closed = false; return async () => { if (closed) return; closed = true; + const errors = []; for (const close of resources) { - await close(); + try { + await close(); + } catch (error) { + errors.push(error); + } + } + + if (errors.length) { + throw new AggregateError(errors, "Failed to close one or more resources"); } }; } @@ -82,21 +91,59 @@ function errorHandler(err, req, res, next) { return next(err); } - const status = Number.isInteger(err.status) ? err.status : 500; + if (err?.type === "entity.parse.failed") { + return res.status(400).json({error: "Invalid JSON"}); + } + + const status = Number.isInteger(err.status) ? err.status : Number.isInteger(err.statusCode) ? err.statusCode : 500; const message = status >= 500 ? "Internal server error" : err.message; - console.warn("REQUEST ERROR", status, req.method, req.url, err); + if (status >= 500) { + console.warn("REQUEST ERROR", status, req.method, req.url, err); + } else { + console.warn("REQUEST ERROR", status, req.method, req.url, message); + } if (wantsJson(req)) { return res.status(status).json({error: message}); } return res.status(status).type("text").send(message); } +function sameOriginProtection(config) { + return (req, res, next) => { + if (!config.sameOriginProtectionEnabled || !["POST", "PUT", "PATCH", "DELETE"].includes(req.method)) { + return next(); + } + + const secFetchSite = req.get("Sec-Fetch-Site"); + if (secFetchSite && !["same-origin", "none"].includes(secFetchSite)) { + return res.status(403).json({error: "Cross-origin request blocked"}); + } + + const origin = req.get("Origin"); + if (!origin) return next(); + + let parsedOrigin; + try { + parsedOrigin = new URL(origin); + } catch { + return res.status(403).json({error: "Cross-origin request blocked"}); + } + + if (parsedOrigin.protocol !== `${req.protocol}:` || parsedOrigin.host !== req.get("Host")) { + return res.status(403).json({error: "Cross-origin request blocked"}); + } + + return next(); + }; +} + 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.dataDir, {recursive: true}); fs.mkdirSync(config.sessionsDir, {recursive: true}); } @@ -116,9 +163,13 @@ export function createApp(options = {}) { ); app.use(express.json({limit: "1mb"})); - app.use(express.static(config.publicDir)); + app.use(express.static(config.publicDir, { + maxAge: config.staticMaxAge, + etag: config.staticEtag, + fallthrough: config.staticFallthrough + })); app.use(session({ - store: config.testMode ? undefined : new FileStore({path: config.sessionsDir, reapInterval: 3600}), + store: options.sessionStore || (config.testMode ? undefined : new FileStore({path: config.sessionsDir, reapInterval: 3600})), secret: config.sessionSecret, resave: false, saveUninitialized: false, @@ -129,6 +180,7 @@ export function createApp(options = {}) { maxAge: config.sessionMaxAgeMs } })); + app.use(sameOriginProtection(config)); app.locals.config = config; app.locals.db = db; @@ -155,8 +207,39 @@ export function startServer(options = {}) { console.log(`Server running at http://localhost:${port}`); }); const close = async () => { - await new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())); - await app.locals.close(); + const errors = []; + try { + await new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())); + } catch (error) { + errors.push(error); + } + try { + await app.locals.close(); + } catch (error) { + if (error instanceof AggregateError) { + errors.push(...error.errors); + } else { + errors.push(error); + } + } + if (errors.length) { + throw new AggregateError(errors, "Failed to close server cleanly"); + } }; + if (options.handleSignals !== false) { + let shuttingDown = false; + const shutdown = (signal) => { + if (shuttingDown) return; + shuttingDown = true; + close() + .then(() => process.exit(0)) + .catch((error) => { + console.error(`Failed to shut down after ${signal}`, error); + process.exit(1); + }); + }; + process.once("SIGINT", shutdown); + process.once("SIGTERM", shutdown); + } return {app, server, close}; } diff --git a/src/config.js b/src/config.js index abc8713..e94d3a5 100644 --- a/src/config.js +++ b/src/config.js @@ -37,6 +37,10 @@ export function createConfig(overrides = {}) { 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), + sameOriginProtectionEnabled: bool(process.env.SAME_ORIGIN_PROTECTION_ENABLED, true), + staticMaxAge: process.env.STATIC_MAX_AGE || (process.env.NODE_ENV === "production" ? "1h" : 0), + staticEtag: bool(process.env.STATIC_ETAG, true), + staticFallthrough: bool(process.env.STATIC_FALLTHROUGH, true), 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), diff --git a/test/app.test.js b/test/app.test.js index ade8046..0cf7aaa 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -1,6 +1,10 @@ import test from "node:test"; import assert from "node:assert/strict"; -import {createApp} from "../src/app.js"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import session from "express-session"; +import {createApp, createCloseHandler} from "../src/app.js"; class CookieJar { constructor() { @@ -55,7 +59,8 @@ async function startTestServer(overrides = {}) { sessionSecret: "test-secret", ...(overrides.config || {}) }, - modelService + modelService, + sessionStore: overrides.sessionStore }); const server = await new Promise((resolve) => { @@ -118,6 +123,140 @@ test("auth flow validates inputs and persists session", async () => { await server.close(); }); +test("createApp can use an injected session store", async () => { + class CountingStore extends session.Store { + constructor() { + super(); + this.sessions = new Map(); + this.setCount = 0; + } + + get(sid, callback) { + callback(null, this.sessions.get(sid)); + } + + set(sid, value, callback) { + this.setCount += 1; + this.sessions.set(sid, value); + callback(); + } + + destroy(sid, callback) { + this.sessions.delete(sid); + callback(); + } + } + + const store = new CountingStore(); + const server = await startTestServer({sessionStore: store}); + const jar = new CookieJar(); + + await request(server.baseUrl, "/register", { + method: "POST", + body: {username: "alice", password: "password123"} + }).then(({status}) => assert.equal(status, 200)); + + await request(server.baseUrl, "/login", { + method: "POST", + body: {username: "alice", password: "password123"}, + jar + }).then(({status}) => assert.equal(status, 200)); + + assert.equal(store.setCount > 0, true); + await server.close(); +}); + +test("invalid JSON returns a clean 400 response", async () => { + const server = await startTestServer(); + + const response = await fetch(`${server.baseUrl}/register`, { + method: "POST", + headers: {"Content-Type": "application/json"}, + body: "{" + }); + const json = await response.json(); + + assert.equal(response.status, 400); + assert.deepEqual(json, {error: "Invalid JSON"}); + await server.close(); +}); + +test("cross-origin state-changing requests are blocked", async () => { + const server = await startTestServer(); + + const response = await fetch(`${server.baseUrl}/register`, { + method: "POST", + headers: { + "Content-Type": "application/json", + Origin: "https://example.test" + }, + body: JSON.stringify({username: "alice", password: "password123"}) + }); + const json = await response.json(); + + assert.equal(response.status, 403); + assert.deepEqual(json, {error: "Cross-origin request blocked"}); + await server.close(); +}); + +test("file-backed startup creates data and session directories", async () => { + const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "krishd-app-")); + const dataDir = path.join(rootDir, "data"); + const sessionsDir = path.join(rootDir, "sessions"); + const app = createApp({ + config: { + rootDir, + dataDir, + sessionsDir, + dbPath: path.join(dataDir, "app.db"), + testMode: true, + sessionSecret: "test-secret" + }, + modelService: { + async generateReply() {}, + async streamReply() {}, + async listModels() { + return {models: []}; + }, + async checkHealth() { + return {ok: true, checkedAt: new Date().toISOString(), error: null}; + }, + mapError(error) { + return {status: 500, body: {error: error.message}}; + } + } + }); + + assert.equal(fs.existsSync(dataDir), true); + assert.equal(fs.existsSync(sessionsDir), true); + await app.locals.close(); +}); + +test("close handler attempts all resources and reports collected failures", async () => { + const closed = []; + const close = createCloseHandler([ + () => { + closed.push("first"); + throw new Error("first failed"); + }, + () => { + closed.push("second"); + }, + () => { + closed.push("third"); + throw new Error("third failed"); + } + ]); + + await assert.rejects(close, (error) => { + assert.equal(error instanceof AggregateError, true); + assert.equal(error.errors.length, 2); + assert.deepEqual(error.errors.map((item) => item.message), ["first failed", "third failed"]); + return true; + }); + assert.deepEqual(closed, ["first", "second", "third"]); +}); + test("chat creation, send, and retry endpoints work with stubbed model service", async () => { const server = await startTestServer(); const jar = new CookieJar();