Skip to content

Commit

Permalink
🛡️ : Security Enhancements (#1681)
Browse files Browse the repository at this point in the history
* fix: sanitize HTTP params and do not send whole error objects backs

* fix: prevent path traversal

* fix: send custom error message for tokenizer route

* chore: handle info exposure vector

* chore(oauth): skip check due to false positive as oauth routes are rate-limited

* chore(app): disable `x-powered-by`

* chore: disable false positives or flagging of hardcoded secrets when they are fake values

* chore: add path traversal safety check
  • Loading branch information
danny-avila authored Jan 30, 2024
1 parent 9fad1b2 commit 972402e
Show file tree
Hide file tree
Showing 23 changed files with 72 additions and 28 deletions.
1 change: 1 addition & 0 deletions api/app/clients/tools/util/handleTools.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ describe('Tool Handlers', () => {
username: 'fakeuser',
email: '[email protected]',
emailVerified: false,
// file deepcode ignore NoHardcodedPasswords/test: fake value
password: 'fakepassword123',
avatar: '',
provider: 'local',
Expand Down
15 changes: 8 additions & 7 deletions api/server/controllers/ErrorController.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,24 @@ const { logger } = require('~/config');
//handle duplicates
const handleDuplicateKeyError = (err, res) => {
logger.error('Duplicate key error:', err.keyValue);
const field = Object.keys(err.keyValue);
const field = `${JSON.stringify(Object.keys(err.keyValue))}`;
const code = 409;
const error = `An document with that ${field} already exists.`;
res.status(code).send({ messages: error, fields: field });
res
.status(code)
.send({ messages: `An document with that ${field} already exists.`, fields: field });
};

//handle validation errors
const handleValidationError = (err, res) => {
logger.error('Validation error:', err.errors);
let errors = Object.values(err.errors).map((el) => el.message);
let fields = Object.values(err.errors).map((el) => el.path);
let fields = `${JSON.stringify(Object.values(err.errors).map((el) => el.path))}`;
let code = 400;
if (errors.length > 1) {
const formattedErrors = errors.join(' ');
res.status(code).send({ messages: formattedErrors, fields: fields });
errors = errors.join(' ');
res.status(code).send({ messages: `${JSON.stringify(errors)}`, fields: fields });
} else {
res.status(code).send({ messages: errors, fields: fields });
res.status(code).send({ messages: `${JSON.stringify(errors)}`, fields: fields });
}
};

Expand Down
1 change: 1 addition & 0 deletions api/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const startServer = async () => {
await indexSync();

const app = express();
app.disable('x-powered-by');
await AppService(app);

app.get('/health', (_req, res) => res.status(200).send('OK'));
Expand Down
2 changes: 2 additions & 0 deletions api/server/routes/__tests__/config.spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const request = require('supertest');
const express = require('express');
const routes = require('../');
// file deepcode ignore UseCsurfForExpress/test: test
const app = express();
app.disable('x-powered-by');
app.use('/api/config', routes.config);

afterEach(() => {
Expand Down
20 changes: 13 additions & 7 deletions api/server/routes/convos.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
const express = require('express');
const router = express.Router();
const { getConvosByPage, deleteConvos } = require('~/models/Conversation');
const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
const { getConvo, saveConvo } = require('~/models');
const { logger } = require('~/config');

const router = express.Router();
router.use(requireJwtAuth);

router.get('/', async (req, res) => {
const pageNumber = req.query.pageNumber || 1;
let pageNumber = req.query.pageNumber || 1;
pageNumber = parseInt(pageNumber, 10);

if (isNaN(pageNumber) || pageNumber < 1) {
return res.status(400).json({ error: 'Invalid page number' });
}

res.status(200).send(await getConvosByPage(req.user.id, pageNumber));
});

Expand All @@ -17,7 +23,7 @@ router.get('/:conversationId', async (req, res) => {
const convo = await getConvo(req.user.id, conversationId);

if (convo) {
res.status(200).send(convo);
res.status(200).json(convo);
} else {
res.status(404).end();
}
Expand All @@ -39,10 +45,10 @@ router.post('/clear', async (req, res) => {

try {
const dbResponse = await deleteConvos(req.user.id, filter);
res.status(201).send(dbResponse);
res.status(201).json(dbResponse);
} catch (error) {
logger.error('Error clearing conversations', error);
res.status(500).send(error);
res.status(500).send('Error clearing conversations');
}
});

Expand All @@ -51,10 +57,10 @@ router.post('/update', async (req, res) => {

try {
const dbResponse = await saveConvo(req.user.id, update);
res.status(201).send(dbResponse);
res.status(201).json(dbResponse);
} catch (error) {
logger.error('Error updating conversation', error);
res.status(500).send(error);
res.status(500).send('Error updating conversation');
}
});

Expand Down
8 changes: 7 additions & 1 deletion api/server/routes/files/images.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { z } = require('zod');
const path = require('path');
const fs = require('fs').promises;
const express = require('express');
const upload = require('./multer');
Expand Down Expand Up @@ -39,7 +40,12 @@ router.post('/', upload.single('file'), async (req, res) => {
} catch (error) {
logger.error('[/files/images] Error processing file:', error);
try {
await fs.unlink(file.path);
const filepath = path.join(
req.app.locals.paths.imageOutput,
req.user.id,
path.basename(file.filename),
);
await fs.unlink(filepath);
} catch (error) {
logger.error('[/files/images] Error deleting file:', error);
}
Expand Down
2 changes: 1 addition & 1 deletion api/server/routes/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ router.put('/:conversationId/:messageId', validateMessageReq, async (req, res) =
const { messageId, model } = req.params;
const { text } = req.body;
const tokenCount = await countTokens(text, model);
res.status(201).send(await updateMessage({ messageId, text, tokenCount }));
res.status(201).json(await updateMessage({ messageId, text, tokenCount }));
});

// DELETE
Expand Down
2 changes: 2 additions & 0 deletions api/server/routes/oauth.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// file deepcode ignore NoRateLimitingForLogin: Rate limiting is handled by the `loginLimiter` middleware

const passport = require('passport');
const express = require('express');
const router = express.Router();
Expand Down
17 changes: 9 additions & 8 deletions api/server/routes/presets.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,28 @@ const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
const { logger } = require('~/config');

const router = express.Router();
router.use(requireJwtAuth);

router.get('/', requireJwtAuth, async (req, res) => {
router.get('/', async (req, res) => {
const presets = (await getPresets(req.user.id)).map((preset) => preset);
res.status(200).send(presets);
res.status(200).json(presets);
});

router.post('/', requireJwtAuth, async (req, res) => {
router.post('/', async (req, res) => {
const update = req.body || {};

update.presetId = update?.presetId || crypto.randomUUID();

try {
const preset = await savePreset(req.user.id, update);
res.status(201).send(preset);
res.status(201).json(preset);
} catch (error) {
logger.error('[/presets] error saving preset', error);
res.status(500).send(error);
res.status(500).send('There was an error when saving the preset');
}
});

router.post('/delete', requireJwtAuth, async (req, res) => {
router.post('/delete', async (req, res) => {
let filter = {};
const { presetId } = req.body || {};

Expand All @@ -37,10 +38,10 @@ router.post('/delete', requireJwtAuth, async (req, res) => {

try {
const deleteCount = await deletePresets(req.user.id, filter);
res.status(201).send(deleteCount);
res.status(201).json(deleteCount);
} catch (error) {
logger.error('[/presets/delete] error deleting presets', error);
res.status(500).send(error);
res.status(500).send('There was an error deleting the presets');
}
});

Expand Down
2 changes: 1 addition & 1 deletion api/server/routes/tokenizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ router.post('/', requireJwtAuth, async (req, res) => {
res.send({ count });
} catch (e) {
logger.error('[/tokenizer] Error counting tokens', e);
res.status(500).send(e.message);
res.status(500).json('Error counting tokens');
}
});

Expand Down
2 changes: 2 additions & 0 deletions api/server/services/Endpoints/google/initializeClient.spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets

const initializeClient = require('./initializeClient');
const { GoogleClient } = require('~/app');
const { checkUserKeyExpiry, getUserKey } = require('../../UserService');
Expand Down
2 changes: 1 addition & 1 deletion api/server/services/Files/Local/crud.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ async function saveFileFromURL({ userId, URL, fileName, basePath = 'images' }) {
}

// Create a writable stream for the output path
const outputFilePath = path.join(outputPath, fileName);
const outputFilePath = path.join(outputPath, path.basename(fileName));
const writer = fs.createWriteStream(outputFilePath);

// Pipe the response data to the output file
Expand Down
2 changes: 2 additions & 0 deletions api/strategies/validators.spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// file deepcode ignore NoHardcodedPasswords: No hard-coded passwords in tests

const { loginSchema, registerSchema, errorsToString } = require('./validators');

describe('Zod Schemas', () => {
Expand Down
2 changes: 2 additions & 0 deletions client/src/components/Messages/Content/Error.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets

import React from 'react';
import type { TOpenAIMessage } from 'librechat-data-provider';
import { formatJSON, extractJson, isJson } from '~/utils/json';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('PluginAuthForm', () => {
action: 'install',
auth: {
key: '1234567890',
// file deepcode ignore HardcodedNonCryptoSecret/test: test
secret: '1234567890',
},
});
Expand Down
2 changes: 2 additions & 0 deletions client/src/localization/languages/Br.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Portuguese phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: 'Exemplos',
Expand Down
2 changes: 2 additions & 0 deletions client/src/localization/languages/Eng.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// English phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: 'Examples',
Expand Down
2 changes: 2 additions & 0 deletions client/src/localization/languages/Es.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Spanish phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: 'Ejemplos',
Expand Down
2 changes: 2 additions & 0 deletions client/src/localization/languages/Fr.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// French phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: 'Exemples',
Expand Down
2 changes: 2 additions & 0 deletions client/src/localization/languages/Id.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Indonesia phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: 'Contoh',
Expand Down
2 changes: 2 additions & 0 deletions client/src/localization/languages/It.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Italian phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: 'Esempi',
Expand Down
7 changes: 5 additions & 2 deletions client/src/localization/languages/Jp.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// English phrases
// Japanese phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: '例',
Expand Down Expand Up @@ -266,7 +268,8 @@ export default {
com_nav_welcome_message: 'How can I help you today?',
com_nav_auto_scroll: 'チャットを開いたときに最新まで自動でスクロール',
com_nav_modular_chat: '会話の途中でのエンドポイント切替を有効化',
com_nav_latex_parsing: 'メッセージ内の LaTeX の構文解析 (パフォーマンスに影響する可能性があります。)',
com_nav_latex_parsing:
'メッセージ内の LaTeX の構文解析 (パフォーマンスに影響する可能性があります。)',
com_nav_profile_picture: 'プロフィール画像',
com_nav_change_picture: '画像を変更',
com_nav_plugin_store: 'プラグインストア',
Expand Down
2 changes: 2 additions & 0 deletions client/src/localization/languages/Zh.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Chinese phrases
// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file

export default {
com_ui_examples: '示例',
Expand Down

0 comments on commit 972402e

Please sign in to comment.