From 4cbc481f5809979f5c38dae43ea7ad301d972bc4 Mon Sep 17 00:00:00 2001 From: "mark.gilbert@prominic.net" Date: Wed, 1 Oct 2025 01:38:23 +0000 Subject: [PATCH 1/3] fix: resolve CodeQL security vulnerabilities (type confusion and Helmet misconfiguration) - Add type checking for req.query.lang to prevent type confusion attacks in i18n middleware - Remove explicit disabling of Helmet's contentSecurityPolicy and hsts features - Fixes CodeQL alerts #19, #20 (type confusion) and #23 (insecure Helmet config) --- app.js | 4 ---- config/i18n.js | 13 +++++++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app.js b/app.js index bd2963c..d22c457 100644 --- a/app.js +++ b/app.js @@ -119,8 +119,6 @@ const startServer = async () => { } helmetConfig.contentSecurityPolicy = { directives: cspDirectives }; - } else { - helmetConfig.contentSecurityPolicy = false; } // Configure HSTS if enabled @@ -130,8 +128,6 @@ const startServer = async () => { includeSubDomains: securityConfig.hsts.include_subdomains, preload: securityConfig.hsts.preload, }; - } else { - helmetConfig.hsts = false; } // Configure additional security headers diff --git a/config/i18n.js b/config/i18n.js index 85e1e7a..c76d43b 100644 --- a/config/i18n.js +++ b/config/i18n.js @@ -137,6 +137,12 @@ export const configAwareI18nMiddleware = (req, res, next) => { // Normal auto-detection: Priority: query param > header > configured default let locale = req.query.lang || req.get('Accept-Language') || i18nConfig.default_language; + if (Array.isArray(locale)) { + [locale] = locale; + } else if (typeof locale !== 'string') { + locale = i18nConfig.default_language; + } + // Parse Accept-Language header if present if (locale && locale.includes(',')) { [locale] = locale.split(','); @@ -151,6 +157,13 @@ export const configAwareI18nMiddleware = (req, res, next) => { // Fallback to simple locale detection if config loading fails console.warn('Config loading failed in i18n middleware, using fallback:', error.message); let locale = req.query.lang || req.get('Accept-Language') || defaultLocale; + + if (Array.isArray(locale)) { + [locale] = locale; + } else if (typeof locale !== 'string') { + locale = defaultLocale; + } + if (locale && locale.includes(',')) { [locale] = locale.split(','); } From 5c0924bb070cf6c09a46b268c3675bdd8db360b7 Mon Sep 17 00:00:00 2001 From: "mark.gilbert@prominic.net" Date: Wed, 1 Oct 2025 01:46:49 +0000 Subject: [PATCH 2/3] fix: add CSRF protection and path validation for folder creation - Add lusca CSRF middleware after session setup - Validate constructed folder paths stay within SERVED_DIR in all 3 folder creation endpoints - Fixes CodeQL alerts for missing CSRF and uncontrolled path injection --- app.js | 3 +++ package-lock.json | 21 +++++++++++++++++++++ package.json | 1 + routes/fileServer.js | 21 +++++++++++++++++++++ 4 files changed, 46 insertions(+) diff --git a/app.js b/app.js index d22c457..768d040 100644 --- a/app.js +++ b/app.js @@ -6,6 +6,7 @@ import cors from 'cors'; import compression from 'compression'; import cookieParser from 'cookie-parser'; import session from 'express-session'; +import lusca from 'lusca'; import configLoader from './config/configLoader.js'; import { configAwareI18nMiddleware } from './config/i18n.js'; import { initializeDatabase } from './config/database.js'; @@ -162,6 +163,8 @@ const startServer = async () => { }) ); + app.use(lusca.csrf()); + app.use(morganMiddleware); app.use(rateLimiterMiddleware()); app.use(configAwareI18nMiddleware); diff --git a/package-lock.json b/package-lock.json index dda42e3..dc6f33a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,6 +24,7 @@ "js-yaml": "^4.1.0", "json-merger": "^3.0.0", "jsonwebtoken": "^9.0.2", + "lusca": "^1.7.0", "morgan": "^1.10.1", "multer": "^2.0.2", "mysql2": "^3.15.1", @@ -5692,6 +5693,17 @@ "url": "https://github.com/sponsors/wellwelwel" } }, + "node_modules/lusca": { + "version": "1.7.0", + "resolved": "https://registry.npmjs.org/lusca/-/lusca-1.7.0.tgz", + "integrity": "sha512-msnrplCfY7zaqlZBDEloCIKld+RUeMZVeWzSPaGUKeRXFlruNSdKg2XxCyR+zj6BqzcXhXlRnvcvx6rAGgsvMA==", + "dependencies": { + "tsscmp": "^1.0.5" + }, + "engines": { + "node": ">=0.8.x" + } + }, "node_modules/magic-string": { "version": "0.30.19", "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.30.19.tgz", @@ -8858,6 +8870,15 @@ "dev": true, "license": "0BSD" }, + "node_modules/tsscmp": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz", + "integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==", + "license": "MIT", + "engines": { + "node": ">=0.6.x" + } + }, "node_modules/tunnel-agent": { "version": "0.6.0", "resolved": "https://registry.npmjs.org/tunnel-agent/-/tunnel-agent-0.6.0.tgz", diff --git a/package.json b/package.json index be2e7bc..1652185 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "js-yaml": "^4.1.0", "json-merger": "^3.0.0", "jsonwebtoken": "^9.0.2", + "lusca": "^1.7.0", "morgan": "^1.10.1", "multer": "^2.0.2", "mysql2": "^3.15.1", diff --git a/routes/fileServer.js b/routes/fileServer.js index 0c32c17..92df2b3 100644 --- a/routes/fileServer.js +++ b/routes/fileServer.js @@ -940,6 +940,13 @@ router.post('/folders', authenticateUploads, async (req, res) => { const targetDir = getSecurePath(requestPath); const newFolderPath = join(targetDir, sanitizedFolderName); + if (!newFolderPath.startsWith(SERVED_DIR)) { + return res.status(400).json({ + success: false, + message: 'Invalid folder path', + }); + } + try { await fs.access(newFolderPath); return res.status(400).json({ @@ -1004,6 +1011,13 @@ router.post('*splat/folders', authenticateUploads, async (req, res) => { const targetDir = getSecurePath(requestPath); const newFolderPath = join(targetDir, sanitizedFolderName); + if (!newFolderPath.startsWith(SERVED_DIR)) { + return res.status(400).json({ + success: false, + message: 'Invalid folder path', + }); + } + try { await fs.access(newFolderPath); return res.status(400).json({ @@ -1205,6 +1219,13 @@ router.post('*splat', authenticateUploads, async (req, res, next) => { const targetDir = getSecurePath(requestPath); const newFolderPath = join(targetDir, sanitizedFolderName); + if (!newFolderPath.startsWith(SERVED_DIR)) { + return res.status(400).json({ + success: false, + message: 'Invalid folder path', + }); + } + try { await fs.access(newFolderPath); return res.status(400).json({ From cf5f18026c17043e3db379916e81a96ac0abe4ac Mon Sep 17 00:00:00 2001 From: "mark.gilbert@prominic.net" Date: Wed, 1 Oct 2025 01:52:04 +0000 Subject: [PATCH 3/3] fix: sanitize XSS vulnerability and remove sensitive data from logs - Sanitize requestPath with escapeHtml before injecting into HTML base href - Remove API key names from access logs to prevent clear-text logging of sensitive info - Fixes CodeQL alerts for reflected XSS and clear-text logging --- routes/apiKeys.js | 4 ++-- routes/fileServer.js | 7 ++++++- routes/swagger.js | 1 - 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/routes/apiKeys.js b/routes/apiKeys.js index 69d67b1..2f6dcba 100644 --- a/routes/apiKeys.js +++ b/routes/apiKeys.js @@ -349,7 +349,7 @@ router.delete('/:id', async (req, res) => { await apiKey.destroy(); - logAccess(req, 'API_KEY_DELETED', `id: ${id}, name: ${apiKey.name}`); + logAccess(req, 'API_KEY_DELETED', `id: ${id}`); return res.json({ success: true, @@ -502,7 +502,7 @@ router.put('/:id', async (req, res) => { await apiKey.update(updateData); - logAccess(req, 'API_KEY_UPDATED', `id: ${id}, name: ${apiKey.name}`); + logAccess(req, 'API_KEY_UPDATED', `id: ${id}`); return res.json({ success: true, diff --git a/routes/fileServer.js b/routes/fileServer.js index 92df2b3..2a96440 100644 --- a/routes/fileServer.js +++ b/routes/fileServer.js @@ -4,6 +4,7 @@ import { promises as fs } from 'fs'; import { join, basename, extname } from 'path'; import { Op } from 'sequelize'; import auth from 'basic-auth'; +import escapeHtml from 'escape-html'; import { SERVED_DIR, getSecurePath } from '../config/paths.js'; import { authenticateDownloads, @@ -87,7 +88,11 @@ const handleDirectoryListing = async (req, res, fullPath, requestPath) => { const staticContent = await getStaticContent(fullPath); if (staticContent) { const baseUrl = requestPath.endsWith('/') ? requestPath : `${requestPath}/`; - const contentWithBase = staticContent.replace('', ``); + const escapedBaseUrl = escapeHtml(baseUrl); + const contentWithBase = staticContent.replace( + '', + `` + ); logAccess(req, 'STATIC_PAGE', 'serving static index.html'); return res.send(contentWithBase); } diff --git a/routes/swagger.js b/routes/swagger.js index 6d02f0e..f50c5b0 100644 --- a/routes/swagger.js +++ b/routes/swagger.js @@ -309,7 +309,6 @@ router.post('/user-api-keys/:id/full', async (req, res) => { logger.info('Full API key retrieved for Swagger', { user: decoded.username || decoded.userId, keyId, - keyName: apiKey.name, }); return undefined; } catch (error) {