From 90459116e36997ab75384af55b5c224520745173 Mon Sep 17 00:00:00 2001 From: Spappz <34202141+Spappz@users.noreply.github.com> Date: Fri, 24 Jan 2025 03:35:56 +0000 Subject: [PATCH 01/21] woohoo --- default/config.yaml | 2 ++ src/middleware/whitelist.js | 24 ++++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/default/config.yaml b/default/config.yaml index 4c28044dc..226aa7dad 100644 --- a/default/config.yaml +++ b/default/config.yaml @@ -31,6 +31,8 @@ enableForwardedWhitelist: true whitelist: - ::1 - 127.0.0.1 +# HTML displayed when a connection is blocked. Use "{{ipDetails}}" to print the client's IP. +whitelistErrorMessage: "
If you are the system administrator, add your IP address to the whitelist or disable whitelist mode by editing config.yaml
in the root directory of your installation.
Connection from {{ipDetails}} has been blocked. This attempt has been logged.
" # Toggle basic authentication for endpoints basicAuthMode: false # Basic authentication credentials diff --git a/src/middleware/whitelist.js b/src/middleware/whitelist.js index 9864e19ae..81f3e0b4f 100644 --- a/src/middleware/whitelist.js +++ b/src/middleware/whitelist.js @@ -1,6 +1,7 @@ import path from 'node:path'; import fs from 'node:fs'; import process from 'node:process'; +import Handlebars from 'handlebars'; import ipMatching from 'ip-matching'; import { getIpFromRequest } from '../express-common.js'; @@ -11,6 +12,9 @@ const enableForwardedWhitelist = getConfigValue('enableForwardedWhitelist', fals let whitelist = getConfigValue('whitelist', []); let knownIPs = new Set(); +const DEFAULT_WHITELIST_ERROR_MESSAGE = + 'If you are the system administrator, add your IP address to the whitelist or disable whitelist mode by editing config.yaml
in the root directory of your installation.
Connection from {{ipDetails}} has been blocked. This attempt has been logged.
'; + if (fs.existsSync(whitelistPath)) { try { let whitelistTxt = fs.readFileSync(whitelistPath, 'utf-8'); @@ -55,9 +59,9 @@ export default function whitelistMiddleware(whitelistMode, listen) { return function (req, res, next) { const clientIp = getIpFromRequest(req); const forwardedIp = getForwardedIp(req); + const userAgent = req.headers['user-agent']; if (listen && !knownIPs.has(clientIp)) { - const userAgent = req.headers['user-agent']; console.log(color.yellow(`New connection from ${clientIp}; User Agent: ${userAgent}\n`)); knownIPs.add(clientIp); @@ -76,9 +80,21 @@ export default function whitelistMiddleware(whitelistMode, listen) { || forwardedIp && whitelistMode === true && !whitelist.some(x => ipMatching.matches(forwardedIp, ipMatching.getMatch(x))) ) { // Log the connection attempt with real IP address - const ipDetails = forwardedIp ? `${clientIp} (forwarded from ${forwardedIp})` : clientIp; - console.log(color.red('Forbidden: Connection attempt from ' + ipDetails + '. If you are attempting to connect, please add your IP address in whitelist or disable whitelist mode in config.yaml in root of SillyTavern folder.\n')); - return res.status(403).send('Forbidden: Connection attempt from ' + ipDetails + '. If you are attempting to connect, please add your IP address in whitelist or disable whitelist mode in config.yaml in root of SillyTavern folder.'); + const ipDetails = forwardedIp + ? `${clientIp} (forwarded from ${forwardedIp})` + : clientIp; + const errorMessage = Handlebars.compile( + getConfigValue( + 'whitelistErrorMessage', + DEFAULT_WHITELIST_ERROR_MESSAGE, + ), + ); + console.log( + color.red( + `Blocked connection from ${clientIp}; User Agent: ${userAgent}\n\tTo allow this connection, add its IP address to the whitelist or disable whitelist mode by editing config.yaml in the root directory of your SillyTavern installation.\n`, + ), + ); + return res.status(403).send(errorMessage({ ipDetails })); } next(); }; From 075368b5aec0cdb1d04c372f15bf1301b7ad2e57 Mon Sep 17 00:00:00 2001 From: Spappz <34202141+Spappz@users.noreply.github.com> Date: Fri, 24 Jan 2025 19:56:19 +0000 Subject: [PATCH 02/21] Ensure Handlebars template is only compiled once --- src/middleware/whitelist.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/middleware/whitelist.js b/src/middleware/whitelist.js index 81f3e0b4f..200a359ce 100644 --- a/src/middleware/whitelist.js +++ b/src/middleware/whitelist.js @@ -15,6 +15,13 @@ let knownIPs = new Set(); const DEFAULT_WHITELIST_ERROR_MESSAGE = 'If you are the system administrator, add your IP address to the whitelist or disable whitelist mode by editing config.yaml
in the root directory of your installation.
Connection from {{ipDetails}} has been blocked. This attempt has been logged.
'; +const errorMessage = Handlebars.compile( + getConfigValue( + 'whitelistErrorMessage', + DEFAULT_WHITELIST_ERROR_MESSAGE, + ), +); + if (fs.existsSync(whitelistPath)) { try { let whitelistTxt = fs.readFileSync(whitelistPath, 'utf-8'); @@ -83,12 +90,6 @@ export default function whitelistMiddleware(whitelistMode, listen) { const ipDetails = forwardedIp ? `${clientIp} (forwarded from ${forwardedIp})` : clientIp; - const errorMessage = Handlebars.compile( - getConfigValue( - 'whitelistErrorMessage', - DEFAULT_WHITELIST_ERROR_MESSAGE, - ), - ); console.log( color.red( `Blocked connection from ${clientIp}; User Agent: ${userAgent}\n\tTo allow this connection, add its IP address to the whitelist or disable whitelist mode by editing config.yaml in the root directory of your SillyTavern installation.\n`, From 0937f44f397eaca9c21b5a8334aca54dd3f506cc Mon Sep 17 00:00:00 2001 From: Cohee <18619528+Cohee1207@users.noreply.github.com> Date: Fri, 24 Jan 2025 23:47:32 +0200 Subject: [PATCH 03/21] Validate avatar_url field with a middleware (#3314) * Validate avatar_url field with a middleware * Fix validating wrong endpoint --- src/endpoints/avatars.js | 3 ++- src/endpoints/backgrounds.js | 3 ++- src/endpoints/characters.js | 19 +++++++++-------- src/endpoints/chats.js | 15 +++++++------ src/endpoints/settings.js | 5 +++-- src/middleware/validateFileName.js | 34 ++++++++++++++++++++++++++++++ 6 files changed, 59 insertions(+), 20 deletions(-) create mode 100644 src/middleware/validateFileName.js diff --git a/src/endpoints/avatars.js b/src/endpoints/avatars.js index 73b995ffb..f84527670 100644 --- a/src/endpoints/avatars.js +++ b/src/endpoints/avatars.js @@ -9,6 +9,7 @@ import { sync as writeFileAtomicSync } from 'write-file-atomic'; import { jsonParser, urlencodedParser } from '../express-common.js'; import { AVATAR_WIDTH, AVATAR_HEIGHT } from '../constants.js'; import { getImages, tryParse } from '../util.js'; +import { getFileNameValidationFunction } from '../middleware/validateFileName.js'; export const router = express.Router(); @@ -17,7 +18,7 @@ router.post('/get', jsonParser, function (request, response) { response.send(JSON.stringify(images)); }); -router.post('/delete', jsonParser, function (request, response) { +router.post('/delete', jsonParser, getFileNameValidationFunction('avatar'), function (request, response) { if (!request.body) return response.sendStatus(400); if (request.body.avatar !== sanitize(request.body.avatar)) { diff --git a/src/endpoints/backgrounds.js b/src/endpoints/backgrounds.js index 0638415e6..13705fa7a 100644 --- a/src/endpoints/backgrounds.js +++ b/src/endpoints/backgrounds.js @@ -7,6 +7,7 @@ import sanitize from 'sanitize-filename'; import { jsonParser, urlencodedParser } from '../express-common.js'; import { invalidateThumbnail } from './thumbnails.js'; import { getImages } from '../util.js'; +import { getFileNameValidationFunction } from '../middleware/validateFileName.js'; export const router = express.Router(); @@ -15,7 +16,7 @@ router.post('/all', jsonParser, function (request, response) { response.send(JSON.stringify(images)); }); -router.post('/delete', jsonParser, function (request, response) { +router.post('/delete', jsonParser, getFileNameValidationFunction('bg'), function (request, response) { if (!request.body) return response.sendStatus(400); if (request.body.bg !== sanitize(request.body.bg)) { diff --git a/src/endpoints/characters.js b/src/endpoints/characters.js index 2aa2cca10..c7a2a02d4 100644 --- a/src/endpoints/characters.js +++ b/src/endpoints/characters.js @@ -14,6 +14,7 @@ import jimp from 'jimp'; import { AVATAR_WIDTH, AVATAR_HEIGHT } from '../constants.js'; import { jsonParser, urlencodedParser } from '../express-common.js'; +import { default as validateAvatarUrlMiddleware, getFileNameValidationFunction } from '../middleware/validateFileName.js'; import { deepMerge, humanizedISO8601DateTime, tryParse, extractFileFromZipBuffer, MemoryLimitedMap, getConfigValue } from '../util.js'; import { TavernCardValidator } from '../validator/TavernCardValidator.js'; import { parse, write } from '../character-card-parser.js'; @@ -756,7 +757,7 @@ router.post('/create', urlencodedParser, async function (request, response) { } }); -router.post('/rename', jsonParser, async function (request, response) { +router.post('/rename', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { if (!request.body.avatar_url || !request.body.new_name) { return response.sendStatus(400); } @@ -803,7 +804,7 @@ router.post('/rename', jsonParser, async function (request, response) { } }); -router.post('/edit', urlencodedParser, async function (request, response) { +router.post('/edit', urlencodedParser, validateAvatarUrlMiddleware, async function (request, response) { if (!request.body) { console.error('Error: no response body detected'); response.status(400).send('Error: no response body detected'); @@ -852,7 +853,7 @@ router.post('/edit', urlencodedParser, async function (request, response) { * @param {Object} response - The HTTP response object. * @returns {void} */ -router.post('/edit-attribute', jsonParser, async function (request, response) { +router.post('/edit-attribute', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { console.log(request.body); if (!request.body) { console.error('Error: no response body detected'); @@ -898,7 +899,7 @@ router.post('/edit-attribute', jsonParser, async function (request, response) { * * @returns {void} * */ -router.post('/merge-attributes', jsonParser, async function (request, response) { +router.post('/merge-attributes', jsonParser, getFileNameValidationFunction('avatar'), async function (request, response) { try { const update = request.body; const avatarPath = path.join(request.user.directories.characters, update.avatar); @@ -929,7 +930,7 @@ router.post('/merge-attributes', jsonParser, async function (request, response) } }); -router.post('/delete', jsonParser, async function (request, response) { +router.post('/delete', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { if (!request.body || !request.body.avatar_url) { return response.sendStatus(400); } @@ -992,7 +993,7 @@ router.post('/all', jsonParser, async function (request, response) { } }); -router.post('/get', jsonParser, async function (request, response) { +router.post('/get', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { try { if (!request.body) return response.sendStatus(400); const item = request.body.avatar_url; @@ -1011,7 +1012,7 @@ router.post('/get', jsonParser, async function (request, response) { } }); -router.post('/chats', jsonParser, async function (request, response) { +router.post('/chats', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { if (!request.body) return response.sendStatus(400); const characterDirectory = (request.body.avatar_url).replace('.png', ''); @@ -1160,7 +1161,7 @@ router.post('/import', urlencodedParser, async function (request, response) { } }); -router.post('/duplicate', jsonParser, async function (request, response) { +router.post('/duplicate', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { try { if (!request.body.avatar_url) { console.log('avatar URL not found in request body'); @@ -1207,7 +1208,7 @@ router.post('/duplicate', jsonParser, async function (request, response) { } }); -router.post('/export', jsonParser, async function (request, response) { +router.post('/export', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { try { if (!request.body.format || !request.body.avatar_url) { return response.sendStatus(400); diff --git a/src/endpoints/chats.js b/src/endpoints/chats.js index 2df194797..c3bab87ac 100644 --- a/src/endpoints/chats.js +++ b/src/endpoints/chats.js @@ -9,6 +9,7 @@ import { sync as writeFileAtomicSync } from 'write-file-atomic'; import _ from 'lodash'; import { jsonParser, urlencodedParser } from '../express-common.js'; +import validateAvatarUrlMiddleware from '../middleware/validateFileName.js'; import { getConfigValue, humanizedISO8601DateTime, @@ -294,7 +295,7 @@ function importRisuChat(userName, characterName, jsonData) { export const router = express.Router(); -router.post('/save', jsonParser, function (request, response) { +router.post('/save', jsonParser, validateAvatarUrlMiddleware, function (request, response) { try { const directoryName = String(request.body.avatar_url).replace('.png', ''); const chatData = request.body.chat; @@ -310,7 +311,7 @@ router.post('/save', jsonParser, function (request, response) { } }); -router.post('/get', jsonParser, function (request, response) { +router.post('/get', jsonParser, validateAvatarUrlMiddleware, function (request, response) { try { const dirName = String(request.body.avatar_url).replace('.png', ''); const directoryPath = path.join(request.user.directories.chats, dirName); @@ -347,7 +348,7 @@ router.post('/get', jsonParser, function (request, response) { }); -router.post('/rename', jsonParser, async function (request, response) { +router.post('/rename', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { if (!request.body || !request.body.original_file || !request.body.renamed_file) { return response.sendStatus(400); } @@ -372,7 +373,7 @@ router.post('/rename', jsonParser, async function (request, response) { return response.send({ ok: true, sanitizedFileName }); }); -router.post('/delete', jsonParser, function (request, response) { +router.post('/delete', jsonParser, validateAvatarUrlMiddleware, function (request, response) { const dirName = String(request.body.avatar_url).replace('.png', ''); const fileName = String(request.body.chatfile); const filePath = path.join(request.user.directories.chats, dirName, sanitize(fileName)); @@ -388,7 +389,7 @@ router.post('/delete', jsonParser, function (request, response) { return response.send('ok'); }); -router.post('/export', jsonParser, async function (request, response) { +router.post('/export', jsonParser, validateAvatarUrlMiddleware, async function (request, response) { if (!request.body.file || (!request.body.avatar_url && request.body.is_group === false)) { return response.sendStatus(400); } @@ -478,7 +479,7 @@ router.post('/group/import', urlencodedParser, function (request, response) { } }); -router.post('/import', urlencodedParser, function (request, response) { +router.post('/import', urlencodedParser, validateAvatarUrlMiddleware, function (request, response) { if (!request.body) return response.sendStatus(400); const format = request.body.file_type; @@ -626,7 +627,7 @@ router.post('/group/save', jsonParser, (request, response) => { return response.send({ ok: true }); }); -router.post('/search', jsonParser, function (request, response) { +router.post('/search', jsonParser, validateAvatarUrlMiddleware, function (request, response) { try { const { query, avatar_url, group_id } = request.body; let chatFiles = []; diff --git a/src/endpoints/settings.js b/src/endpoints/settings.js index 3cc7e3bec..4df4978cc 100644 --- a/src/endpoints/settings.js +++ b/src/endpoints/settings.js @@ -9,6 +9,7 @@ import { SETTINGS_FILE } from '../constants.js'; import { getConfigValue, generateTimestamp, removeOldBackups } from '../util.js'; import { jsonParser } from '../express-common.js'; import { getAllUserHandles, getUserDirectories } from '../users.js'; +import { getFileNameValidationFunction } from '../middleware/validateFileName.js'; const ENABLE_EXTENSIONS = !!getConfigValue('extensions.enabled', true); const ENABLE_EXTENSIONS_AUTO_UPDATE = !!getConfigValue('extensions.autoUpdate', true); @@ -296,7 +297,7 @@ router.post('/get-snapshots', jsonParser, async (request, response) => { } }); -router.post('/load-snapshot', jsonParser, async (request, response) => { +router.post('/load-snapshot', jsonParser, getFileNameValidationFunction('name'), async (request, response) => { try { const userFilesPattern = getFilePrefix(request.user.profile.handle); @@ -330,7 +331,7 @@ router.post('/make-snapshot', jsonParser, async (request, response) => { } }); -router.post('/restore-snapshot', jsonParser, async (request, response) => { +router.post('/restore-snapshot', jsonParser, getFileNameValidationFunction('name'), async (request, response) => { try { const userFilesPattern = getFilePrefix(request.user.profile.handle); diff --git a/src/middleware/validateFileName.js b/src/middleware/validateFileName.js new file mode 100644 index 000000000..ccfb0e88d --- /dev/null +++ b/src/middleware/validateFileName.js @@ -0,0 +1,34 @@ +import path from 'node:path'; + +/** + * Gets a middleware function that validates the field in the request body. + * @param {string} fieldName Field name + * @returns {import('express').RequestHandler} Middleware function + */ +export function getFileNameValidationFunction(fieldName) { + /** + * Validates the field in the request body. + * @param {import('express').Request} req Request object + * @param {import('express').Response} res Response object + * @param {import('express').NextFunction} next Next middleware + */ + return function validateAvatarUrlMiddleware(req, res, next) { + if (req.body && fieldName in req.body && typeof req.body[fieldName] === 'string') { + const forbiddenRegExp = path.sep === '/' ? /[/\x00]/ : /[/\x00\\]/; + if (forbiddenRegExp.test(req.body[fieldName])) { + console.error('An error occurred while validating the request body', { + handle: req.user.profile.handle, + path: req.originalUrl, + field: fieldName, + value: req.body[fieldName], + }); + return res.sendStatus(400); + } + } + + next(); + }; +} + +const avatarUrlValidationFunction = getFileNameValidationFunction('avatar_url'); +export default avatarUrlValidationFunction; From 928487985d56043a1db3bb54d41657db2966c1b3 Mon Sep 17 00:00:00 2001 From: Spappz <34202141+Spappz@users.noreply.github.com> Date: Sat, 25 Jan 2025 03:38:52 +0000 Subject: [PATCH 04/21] defer 403 HTML to file --- default/config.yaml | 2 -- .../public/error/forbidden-by-whitelist.html | 22 +++++++++++++++++++ src/middleware/whitelist.js | 13 ++--------- 3 files changed, 24 insertions(+), 13 deletions(-) create mode 100644 default/public/error/forbidden-by-whitelist.html diff --git a/default/config.yaml b/default/config.yaml index 226aa7dad..4c28044dc 100644 --- a/default/config.yaml +++ b/default/config.yaml @@ -31,8 +31,6 @@ enableForwardedWhitelist: true whitelist: - ::1 - 127.0.0.1 -# HTML displayed when a connection is blocked. Use "{{ipDetails}}" to print the client's IP. -whitelistErrorMessage: "If you are the system administrator, add your IP address to the whitelist or disable whitelist mode by editing config.yaml
in the root directory of your installation.
Connection from {{ipDetails}} has been blocked. This attempt has been logged.
" # Toggle basic authentication for endpoints basicAuthMode: false # Basic authentication credentials diff --git a/default/public/error/forbidden-by-whitelist.html b/default/public/error/forbidden-by-whitelist.html new file mode 100644 index 000000000..70ff71852 --- /dev/null +++ b/default/public/error/forbidden-by-whitelist.html @@ -0,0 +1,22 @@ + + + + +
+ If you are the system administrator, add your IP address to the
+ whitelist or disable whitelist mode by editing
+ config.yaml
in the root directory of your installation.
+
+ Connection from {{ipDetails}} has been blocked. This attempt + has been logged. +
+ + + diff --git a/src/middleware/whitelist.js b/src/middleware/whitelist.js index 200a359ce..6edf5a75a 100644 --- a/src/middleware/whitelist.js +++ b/src/middleware/whitelist.js @@ -11,16 +11,7 @@ const whitelistPath = path.join(process.cwd(), './whitelist.txt'); const enableForwardedWhitelist = getConfigValue('enableForwardedWhitelist', false); let whitelist = getConfigValue('whitelist', []); let knownIPs = new Set(); - -const DEFAULT_WHITELIST_ERROR_MESSAGE = - 'If you are the system administrator, add your IP address to the whitelist or disable whitelist mode by editing config.yaml
in the root directory of your installation.
Connection from {{ipDetails}} has been blocked. This attempt has been logged.
'; - -const errorMessage = Handlebars.compile( - getConfigValue( - 'whitelistErrorMessage', - DEFAULT_WHITELIST_ERROR_MESSAGE, - ), -); +const forbiddenWebpage = Handlebars.compile(fs.readFileSync('./public/error/forbidden-by-whitelist.html', 'utf-8')); if (fs.existsSync(whitelistPath)) { try { @@ -95,7 +86,7 @@ export default function whitelistMiddleware(whitelistMode, listen) { `Blocked connection from ${clientIp}; User Agent: ${userAgent}\n\tTo allow this connection, add its IP address to the whitelist or disable whitelist mode by editing config.yaml in the root directory of your SillyTavern installation.\n`, ), ); - return res.status(403).send(errorMessage({ ipDetails })); + return res.status(403).send(forbiddenWebpage({ ipDetails })); } next(); }; From 538d66191e9558b4424d07fbf66aac40185f1c98 Mon Sep 17 00:00:00 2001 From: Spappz <34202141+Spappz@users.noreply.github.com> Date: Sat, 25 Jan 2025 03:40:47 +0000 Subject: [PATCH 05/21] add 401 error page for `basicAuth` mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most modern browsers don't actually show users 401 responses, but it doesn't hurt to have it in there anyway ¯\_(ツ)_/¯ --- default/public/error/unauthorized.html | 17 +++++++++++++++++ src/middleware/basicAuth.js | 4 +++- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 default/public/error/unauthorized.html diff --git a/default/public/error/unauthorized.html b/default/public/error/unauthorized.html new file mode 100644 index 000000000..e3fa5f94d --- /dev/null +++ b/default/public/error/unauthorized.html @@ -0,0 +1,17 @@ + + + + +
+ If you are the system administrator, you can configure the
+ basicAuthUser
credentials by editing
+ config.yaml
in the root directory of your installation.
+
+ The requested URL was not found on this server. +
+ + + diff --git a/server.js b/server.js index db7ba9306..bc2a884c2 100644 --- a/server.js +++ b/server.js @@ -615,6 +615,12 @@ app.use('/api/backends/scale-alt', scaleAltRouter); app.use('/api/speech', speechRouter); app.use('/api/azure', azureRouter); +// If all other middlewares fail, send 404 error. +const notFoundWebpage = fs.readFileSync('./public/error/url-not-found.html', { encoding: 'utf-8' }); +app.use((req, res, next) => { + res.status(404).send(notFoundWebpage); +}); + const tavernUrlV6 = new URL( (cliArguments.ssl ? 'https://' : 'http://') + (listen ? '[::]' : '[::1]') + From e07faea874bf201e65aa442a5136dcd27ef8e414 Mon Sep 17 00:00:00 2001 From: Spappz <34202141+Spappz@users.noreply.github.com> Date: Sat, 25 Jan 2025 03:45:16 +0000 Subject: [PATCH 07/21] rework `createDefaultFiles()` Reorganised copy-able `default/` files as a sparse copy of the production file-tree. This should save the `defaultItems` (formerly `files`) array from getting unwieldy. --- .gitignore | 1 + default/{ => public/css}/user.css | 0 post-install.js | 62 +++++++++++++++++++++++++------ 3 files changed, 52 insertions(+), 11 deletions(-) rename default/{ => public/css}/user.css (100%) diff --git a/.gitignore b/.gitignore index 7d48fd879..de09f68ee 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ access.log /vectors/ /cache/ public/css/user.css +public/error/ /plugins/ /data /default/scaffold diff --git a/default/user.css b/default/public/css/user.css similarity index 100% rename from default/user.css rename to default/public/css/user.css diff --git a/post-install.js b/post-install.js index 4c9765eb8..f7863ab66 100644 --- a/post-install.js +++ b/post-install.js @@ -2,7 +2,7 @@ * Scripts to be done before starting the server for the first time. */ import fs from 'node:fs'; -import path from 'node:path'; +import path from 'node:path/posix'; // Windows can handle Unix paths, but not vice-versa import crypto from 'node:crypto'; import process from 'node:process'; import yaml from 'yaml'; @@ -213,20 +213,60 @@ function addMissingConfigValues() { * Creates the default config files if they don't exist yet. */ function createDefaultFiles() { - const files = { - config: './config.yaml', - user: './public/css/user.css', - }; + /** + * @typedef DefaultItem + * @type {object} + * @property {'file' | 'directory'} type - Whether the item should be copied as a single file or merged into a directory structure. + * @property {string} defaultPath - The path to the default item (typically in `default/`). + * @property {string} productionPath - The path to the copied item for production use. + */ - for (const file of Object.values(files)) { + /** @type {DefaultItem[]} */ + const defaultItems = [ + { + type: 'file', + defaultPath: './default/config.yaml', + productionPath: './config.yaml', + }, + { + type: 'directory', + defaultPath: './default/public/', + productionPath: './public/', + }, + ]; + + for (const defaultItem of defaultItems) { try { - if (!fs.existsSync(file)) { - const defaultFilePath = path.join('./default', path.parse(file).base); - fs.copyFileSync(defaultFilePath, file); - console.log(color.green(`Created default file: ${file}`)); + if (defaultItem.type === 'file') { + if (!fs.existsSync(defaultItem.productionPath)) { + fs.copyFileSync( + defaultItem.defaultPath, + defaultItem.productionPath, + ); + console.log( + color.green(`Created default file: ${defaultItem.productionPath}`), + ); + } + } else if (defaultItem.type === 'directory') { + fs.cpSync(defaultItem.defaultPath, defaultItem.productionPath, { + force: false, // Don't overwrite existing files! + recursive: true, + }); + console.log( + color.green(`Synchronized missing files: ${defaultItem.productionPath}`), + ); + } else { + throw new Error( + 'FATAL: Unexpected default file format in `post-install.js#createDefaultFiles()`.', + ); } } catch (error) { - console.error(color.red(`FATAL: Could not write default file: ${file}`), error); + console.error( + color.red( + `FATAL: Could not write default ${defaultItem.type}: ${defaultItem.productionPath}`, + ), + error, + ); } } } From 5ff402aabfab1146802ecd7d18112f1e5933d11e Mon Sep 17 00:00:00 2001 From: Cohee <18619528+Cohee1207@users.noreply.github.com> Date: Sat, 25 Jan 2025 16:56:11 +0200 Subject: [PATCH 08/21] Replace CSRF middleware Closes #3349 --- default/config.yaml | 2 +- package-lock.json | 10 ++++----- package.json | 2 +- server.js | 41 +++++++++++++++++++--------------- src/endpoints/users-private.js | 1 + 5 files changed, 31 insertions(+), 25 deletions(-) diff --git a/default/config.yaml b/default/config.yaml index 4c28044dc..f3e7db625 100644 --- a/default/config.yaml +++ b/default/config.yaml @@ -70,7 +70,7 @@ perUserBasicAuth: false ## Set to a positive number to expire session after a certain time of inactivity ## Set to 0 to expire session when the browser is closed ## Set to a negative number to disable session expiration -sessionTimeout: 86400 +sessionTimeout: -1 # Used to sign session cookies. Will be auto-generated if not set cookieSecret: '' # Disable CSRF protection - NOT RECOMMENDED diff --git a/package-lock.json b/package-lock.json index 5784a9946..4e79f9f50 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26,7 +26,7 @@ "cookie-parser": "^1.4.6", "cookie-session": "^2.1.0", "cors": "^2.8.5", - "csrf-csrf": "^2.2.3", + "csrf-sync": "^4.0.3", "diff-match-patch": "^1.0.5", "dompurify": "^3.1.7", "droll": "^0.2.1", @@ -2987,10 +2987,10 @@ "node": "*" } }, - "node_modules/csrf-csrf": { - "version": "2.2.4", - "resolved": "https://registry.npmjs.org/csrf-csrf/-/csrf-csrf-2.2.4.tgz", - "integrity": "sha512-LuhBmy5RfRmEfeqeYqgaAuS1eDpVtKZB/Eiec9xiKQLBynJxrGVRdM2yRT/YMl1Njo/yKh2L9AYsIwSlTPnx2A==", + "node_modules/csrf-sync": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/csrf-sync/-/csrf-sync-4.0.3.tgz", + "integrity": "sha512-wXzltBBzt/7imzDt6ZT7G/axQG7jo4Sm0uXDUzFY8hR59qhDHdjqpW2hojS4oAVIZDzwlMQloIVCTJoDDh0wwA==", "license": "ISC", "dependencies": { "http-errors": "^2.0.0" diff --git a/package.json b/package.json index 672ffada5..b14a487b8 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "cookie-parser": "^1.4.6", "cookie-session": "^2.1.0", "cors": "^2.8.5", - "csrf-csrf": "^2.2.3", + "csrf-sync": "^4.0.3", "diff-match-patch": "^1.0.5", "dompurify": "^3.1.7", "droll": "^0.2.1", diff --git a/server.js b/server.js index db7ba9306..26dcb4e0d 100644 --- a/server.js +++ b/server.js @@ -18,10 +18,9 @@ import { hideBin } from 'yargs/helpers'; // express/server related library imports import cors from 'cors'; -import { doubleCsrf } from 'csrf-csrf'; +import { csrfSync } from 'csrf-sync'; import express from 'express'; import compression from 'compression'; -import cookieParser from 'cookie-parser'; import cookieSession from 'cookie-session'; import multer from 'multer'; import responseTime from 'response-time'; @@ -40,7 +39,6 @@ util.inspect.defaultOptions.depth = 4; import { loadPlugins } from './src/plugin-loader.js'; import { initUserStorage, - getCsrfSecret, getCookieSecret, getCookieSessionName, getAllEnabledUsers, @@ -347,8 +345,8 @@ if (enableCorsProxy) { } function getSessionCookieAge() { - // Defaults to 24 hours in seconds if not set - const configValue = getConfigValue('sessionTimeout', 24 * 60 * 60); + // Defaults to "no expiration" if not set + const configValue = getConfigValue('sessionTimeout', -1); // Convert to milliseconds if (configValue > 0) { @@ -377,27 +375,34 @@ app.use(setUserDataMiddleware); // CSRF Protection // if (!disableCsrf) { - const COOKIES_SECRET = getCookieSecret(); - - const { generateToken, doubleCsrfProtection } = doubleCsrf({ - getSecret: getCsrfSecret, - cookieName: 'X-CSRF-Token', - cookieOptions: { - sameSite: 'strict', - secure: false, + const csrfSyncProtection = csrfSync({ + getTokenFromState: (req) => { + if (!req.session) { + console.error('(CSRF error) getTokenFromState: Session object not initialized'); + return; + } + return req.session.csrfToken; }, - size: 64, - getTokenFromRequest: (req) => req.headers['x-csrf-token'], + getTokenFromRequest: (req) => { + return req.headers['x-csrf-token']?.toString(); + }, + storeTokenInState: (req, token) => { + if (!req.session) { + console.error('(CSRF error) storeTokenInState: Session object not initialized'); + return; + } + req.session.csrfToken = token; + }, + size: 32, }); app.get('/csrf-token', (req, res) => { res.json({ - 'token': generateToken(res, req), + 'token': csrfSyncProtection.generateToken(req), }); }); - app.use(cookieParser(COOKIES_SECRET)); - app.use(doubleCsrfProtection); + app.use(csrfSyncProtection.csrfSynchronisedProtection); } else { console.warn('\nCSRF protection is disabled. This will make your server vulnerable to CSRF attacks.\n'); app.get('/csrf-token', (req, res) => { diff --git a/src/endpoints/users-private.js b/src/endpoints/users-private.js index 81b60acea..1fde87083 100644 --- a/src/endpoints/users-private.js +++ b/src/endpoints/users-private.js @@ -23,6 +23,7 @@ router.post('/logout', async (request, response) => { } request.session.handle = null; + request.session.csrfToken = null; request.session = null; return response.sendStatus(204); } catch (error) { From 2d8da60ffc6f20c2a330757f40a1633fdb7d51ef Mon Sep 17 00:00:00 2001 From: Cohee <18619528+Cohee1207@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:13:25 +0200 Subject: [PATCH 09/21] Fix types for session --- index.d.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/index.d.ts b/index.d.ts index 015d8e353..35f34c22c 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1,6 +1,24 @@ import { UserDirectoryList, User } from "./src/users"; +import { CsrfSyncedToken } from "csrf-sync"; declare global { + declare namespace CookieSessionInterfaces { + export interface CookieSessionObject { + /** + * The CSRF token for the session. + */ + csrfToken: CsrfSyncedToken; + /** + * Authenticated user handle. + */ + handle: string; + /** + * Last time the session was extended. + */ + touch: number; + } + } + namespace Express { export interface Request { user: { @@ -15,11 +33,3 @@ declare global { */ var DATA_ROOT: string; } - -declare module 'express-session' { - export interface SessionData { - handle: string; - touch: number; - // other properties... - } - } From 9e54070c1d6176fe4fa2e21e523ba14507d0349f Mon Sep 17 00:00:00 2001 From: Spappz <34202141+Spappz@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:38:01 +0000 Subject: [PATCH 10/21] Revert `path/posix` to `path` in `post-install.js` --- post-install.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/post-install.js b/post-install.js index f7863ab66..46ad160cb 100644 --- a/post-install.js +++ b/post-install.js @@ -2,7 +2,7 @@ * Scripts to be done before starting the server for the first time. */ import fs from 'node:fs'; -import path from 'node:path/posix'; // Windows can handle Unix paths, but not vice-versa +import path from 'node:path'; import crypto from 'node:crypto'; import process from 'node:process'; import yaml from 'yaml'; From 6099ffece134bcde9197c1be51a0b23f885465b5 Mon Sep 17 00:00:00 2001 From: Spappz <34202141+Spappz@users.noreply.github.com> Date: Sat, 25 Jan 2025 20:29:31 +0000 Subject: [PATCH 11/21] No exceptions on missing error webpages - Create a `safeReadFileSync()` function in `src/utils.js` to wrap around `fs.readFileSync()` - Migrate error-webpage loads to use `safeReadFileSync()`, with default values of an empty string - Move the 404 error middleware to explicitly only be called *after* extensions are registered --- server.js | 18 ++++++++++++------ src/middleware/basicAuth.js | 5 ++--- src/middleware/whitelist.js | 6 ++++-- src/util.js | 11 +++++++++++ 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/server.js b/server.js index bc2a884c2..cbd161847 100644 --- a/server.js +++ b/server.js @@ -67,6 +67,7 @@ import { forwardFetchResponse, removeColorFormatting, getSeparator, + safeReadFileSync, } from './src/util.js'; import { UPLOADS_DIRECTORY } from './src/constants.js'; import { ensureThumbnailCache } from './src/endpoints/thumbnails.js'; @@ -615,12 +616,6 @@ app.use('/api/backends/scale-alt', scaleAltRouter); app.use('/api/speech', speechRouter); app.use('/api/azure', azureRouter); -// If all other middlewares fail, send 404 error. -const notFoundWebpage = fs.readFileSync('./public/error/url-not-found.html', { encoding: 'utf-8' }); -app.use((req, res, next) => { - res.status(404).send(notFoundWebpage); -}); - const tavernUrlV6 = new URL( (cliArguments.ssl ? 'https://' : 'http://') + (listen ? '[::]' : '[::1]') + @@ -927,6 +922,16 @@ async function verifySecuritySettings() { } } +/** + * Registers a not-found error response if a not-found error page exists. Should only be called after all other middlewares have been registered. + */ +function apply404Middleware() { + const notFoundWebpage = safeReadFileSync('./public/error/url-not-found.html') ?? ''; + app.use((req, res) => { + res.status(404).send(notFoundWebpage); + }); +} + // User storage module needs to be initialized before starting the server initUserStorage(dataRoot) .then(ensurePublicDirectoriesExist) @@ -934,4 +939,5 @@ initUserStorage(dataRoot) .then(migrateSystemPrompts) .then(verifySecuritySettings) .then(preSetupTasks) + .then(apply404Middleware) .finally(startServer); diff --git a/src/middleware/basicAuth.js b/src/middleware/basicAuth.js index 542aad634..4548a3f20 100644 --- a/src/middleware/basicAuth.js +++ b/src/middleware/basicAuth.js @@ -5,13 +5,12 @@ import { Buffer } from 'node:buffer'; import storage from 'node-persist'; import { getAllUserHandles, toKey, getPasswordHash } from '../users.js'; -import { getConfig, getConfigValue } from '../util.js'; -import { readFileSync } from 'node:fs'; +import { getConfig, getConfigValue, safeReadFileSync } from '../util.js'; const PER_USER_BASIC_AUTH = getConfigValue('perUserBasicAuth', false); const ENABLE_ACCOUNTS = getConfigValue('enableUserAccounts', false); -const unauthorizedWebpage = readFileSync('./public/error/unauthorized.html', { encoding: 'utf-8' }); +const unauthorizedWebpage = safeReadFileSync('./public/error/unauthorized.html') ?? ''; const unauthorizedResponse = (res) => { res.set('WWW-Authenticate', 'Basic realm="SillyTavern", charset="UTF-8"'); return res.status(401).send(unauthorizedWebpage); diff --git a/src/middleware/whitelist.js b/src/middleware/whitelist.js index 6edf5a75a..2922c42b1 100644 --- a/src/middleware/whitelist.js +++ b/src/middleware/whitelist.js @@ -5,13 +5,15 @@ import Handlebars from 'handlebars'; import ipMatching from 'ip-matching'; import { getIpFromRequest } from '../express-common.js'; -import { color, getConfigValue } from '../util.js'; +import { color, getConfigValue, safeReadFileSync } from '../util.js'; const whitelistPath = path.join(process.cwd(), './whitelist.txt'); const enableForwardedWhitelist = getConfigValue('enableForwardedWhitelist', false); let whitelist = getConfigValue('whitelist', []); let knownIPs = new Set(); -const forbiddenWebpage = Handlebars.compile(fs.readFileSync('./public/error/forbidden-by-whitelist.html', 'utf-8')); +const forbiddenWebpage = Handlebars.compile( + safeReadFileSync('./public/error/forbidden-by-whitelist.html') ?? '', +); if (fs.existsSync(whitelistPath)) { try { diff --git a/src/util.js b/src/util.js index 0fe03e76c..19bd3ccc7 100644 --- a/src/util.js +++ b/src/util.js @@ -871,3 +871,14 @@ export class MemoryLimitedMap { return this.map[Symbol.iterator](); } } + +/** + * A 'safe' version of `fs.readFileSync()`. Returns the contents of a file if it exists, falling back to a default value if not. + * @param {string} filePath Path of the file to be read. + * @param {Parametersconfig.yaml
for allowed hosts)